Skip to content

Unblock Self-Hosted release by introducing UsageServiceMock #14162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

geropl
Copy link
Member

@geropl geropl commented Oct 25, 2022

Description

Mitigate the fact that usage is not deployed with self-hosted, yet, by mocking it away until we properly fixed it with #14129.

Related Issue(s)

Context: #14129

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-gpl-14129-sh-usage.2 because the annotations in the pull request description changed
(with .werft/ from main)

@geropl
Copy link
Member Author

geropl commented Oct 25, 2022

/werft run with-clean-slate-deployment=true

👍 started the job as gitpod-build-gpl-14129-sh-usage.4
(with .werft/ from main)

@geropl
Copy link
Member Author

geropl commented Oct 26, 2022

/werft run with-preview=true

👍 started the job as gitpod-build-gpl-14129-sh-usage.5
(with .werft/ from main)

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. I didn't think of this approach when I was investigating.

attributionId: AttributionId.render(AttributionId.create(team)),
});
if (costCenter.costCenter?.billingStrategy === CostCenter_BillingStrategy.BILLING_STRATEGY_STRIPE) {
const billingStrategy = await this.usageService.getCurrentBillingStategy(AttributionId.create(team));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talking to the usage service here is okay, because we should only reach this stage when isUsageBasedBillingEnabled == true

if (config.enablePayment) {
return ctx.container.get<UsageServiceImpl>(UsageServiceImpl);
}
return new UsageServiceMock();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Let's call it a NoOpUsageService or something along the lines. Calling it a mock smells like this is test code.

@geropl
Copy link
Member Author

geropl commented Oct 26, 2022

This makes sense. I didn't think of this approach when I was investigating.

It's a hack, honestly, but should work for today. We definitely need the usage service in self-hosted. Crazy how much we depend on it already (and also a warning sign; some of the calls might have better been encapsulated somehow).

@geropl geropl force-pushed the gpl/14129-sh-usage branch from 837a53a to 6da8a9f Compare October 26, 2022 07:36
@geropl
Copy link
Member Author

geropl commented Oct 26, 2022

Supersede by this PR: #14173

@geropl geropl closed this Oct 26, 2022
@geropl geropl deleted the gpl/14129-sh-usage branch January 19, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants