Skip to content

Rename to self-hosted in backend #30556

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 1 commit into from

Conversation

chadwhitacre
Copy link
Member

@chadwhitacre chadwhitacre commented Dec 9, 2021

Part of getsentry/self-hosted#796, split out from #30528 by request.

Follow up is #30558.

@chadwhitacre chadwhitacre requested a review from a team December 9, 2021 21:21
@@ -153,7 +153,7 @@ def get_client_config(request=None):
"statuspage": _get_statuspage(),
"messages": [{"message": msg.message, "level": msg.tags} for msg in messages],
"apmSampling": float(settings.SENTRY_FRONTEND_APM_SAMPLING or 0),
"isOnPremise": settings.SENTRY_ONPREMISE,
"isOnPremise": settings.SENTRY_SELF_HOSTED,
Copy link
Member Author

Choose a reason for hiding this comment

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

On this PR, maintain key name used by frontend.

dashed
dashed previously approved these changes Dec 9, 2021
# reading from that for now, but in the rest of _this_ codebase we should
# reference SENTRY_SELF_HOSTED.
SENTRY_ONPREMISE = True # deprecated, use ...
SENTRY_SELF_HOSTED = SENTRY_ONPREMISE # ... this instead
Copy link
Member Author

@chadwhitacre chadwhitacre Dec 9, 2021

Choose a reason for hiding this comment

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

This is all wrong. I need to actually test this in getsentry. How do I do that?

The result of this PR will be to always set SENTRY_SELF_HOSTED to true, regardless of the value of SENTRY_ONPREMISE in a downstream project. Since SENTRY_SELF_HOSTED then becomes the config passed out to the frontend, if I merge/deploy this then I will effectively turn SaaS into a self-hosted instance.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@chadwhitacre chadwhitacre Dec 9, 2021

Choose a reason for hiding this comment

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

The better way to do this is to:

  1. Set SENTRY_SELF_HOSTED in the downstream apps (getsentry and single-tenant) as a no-op duplicate. It sits right next to SENTRY_ONPREMISE but nothing reads it.
  2. Modify the upstream (i.e., sentry, here) to use/read SENTRY_SELF_HOSTED on the backend (basically this here PR here).
  3. Remove SENTRY_ONPREMISE from the downstreams.
  4. Modify the upstream to set isSelfHosted as a no-op duplicate on the frontend.
  5. Modify the downstreams to read from isSelfHosted.
  6. Modify the upstream to read from isSelfHosted and remove isOnPremise.

The crux is that downstreams set the backend SENTRY_FOO but consume the frontend isFooBar, while upstreams consume the backend and set the frontend. The right approach looks like this:

backend  add    downstream
backend  add    upstream
backend  remove upstream
backend  remove downstream

frontend add    upstream
frontend add    downstream
frontend remove downstream
frontend remove upstream

Copy link
Member Author

Choose a reason for hiding this comment

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

I've folded this back into getsentry/self-hosted#796. To be continued over there ...

@dashed dashed dismissed their stale review December 9, 2021 22:27

Dismissing my approval until the new strategy is in place

@chadwhitacre chadwhitacre deleted the cwlw/rename-to-self-hosted-in-backend branch December 15, 2021 18:54
@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants