Rename onpremise to self-hosted in configs #30528
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,7 +153,8 @@ 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_SELF_HOSTED, # deprecated, use ... | ||
| "isSelfHosted": settings.SENTRY_SELF_HOSTED, # ... this instead | ||
|
There was a problem hiding this comment. Second-riskiest part. There was a problem hiding this comment. Will we be removing There was a problem hiding this comment. I'd like to, but I'm not sure how much impact that has on self-hosters. Probably not much? I'd like to at least get one release out with both keys and a notice in the changelog, then remove "sometime" after that. Thoughts? There was a problem hiding this comment. I'd imagine it shouldn't affect them, if they are customizing their install, they shouldn't have a need to differentiate between the two? But otherwise sounds good There was a problem hiding this comment. I mean the default is true. Nobody except us in SaaS should need to change the default. Also yes, like you say, nobody in the wild building their own plugins or whatever should need to vary their behavior on whether this config is true or false, realistically they would just always deliver whatever behavior they're building for their custom app/plugin. I think I'm convinced that we can remove this sooner rather than later (meaning, at the earliest, after There was a problem hiding this comment. Hmm ... single-tenant, though ... 🤔 There was a problem hiding this comment. Yup, PR needed there as well! There was a problem hiding this comment. Noted on getsentry/self-hosted#796. 👍 |
||
| "invitesEnabled": settings.SENTRY_ENABLE_INVITES, | ||
| "gravatarBaseUrl": settings.SENTRY_GRAVATAR_BASE_URL, | ||
| "termsUrl": settings.TERMS_URL, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,8 @@ def get_default_context(request, existing_context=None, team=None): | |
| "URL_PREFIX": options.get("system.url-prefix"), | ||
| "SINGLE_ORGANIZATION": settings.SENTRY_SINGLE_ORGANIZATION, | ||
| "PLUGINS": plugins, | ||
| "ONPREMISE": settings.SENTRY_SELF_HOSTED, # deprecated, use ... | ||
| "SELF_HOSTED": settings.SENTRY_SELF_HOSTED, # ... this instead | ||
|
There was a problem hiding this comment. What are these contexts? What consumes them? There was a problem hiding this comment. Introduced in #3350 so Django templates can vary: We're not using this anymore, but if we remove this then we should probably remove |
||
| } | ||
|
|
||
| if existing_context: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,12 +7,12 @@ import DefaultSettings from 'sentry/plugins/components/settings'; | |
| type Props = DefaultSettings['props']; | ||
|
|
||
| type State = DefaultSettings['state'] & { | ||
| showSelfHostedConfiguration?: boolean; | ||
|
There was a problem hiding this comment. This ancient plugin has a few config values that are only relevant on self-hosted. This is the source of the screenshots that changed. |
||
| }; | ||
|
|
||
| class Settings extends DefaultSettings<Props, State> { | ||
| REQUIRED_FIELDS = ['account_email', 'api_token', 'website_id']; | ||
| SELF_HOSTED_FIELDS = ['api_url', 'player_url']; | ||
|
|
||
| renderFields(fields: State['fieldList']) { | ||
| return fields?.map(f => | ||
|
|
@@ -29,9 +29,9 @@ class Settings extends DefaultSettings<Props, State> { | |
| return fields?.filter(field => fieldNames.includes(field.name)) ?? []; | ||
| } | ||
|
|
||
| toggleSelfHostedConfiguration = () => { | ||
| this.setState({ | ||
| showSelfHostedConfiguration: !this.state.showSelfHostedConfiguration, | ||
| }); | ||
| }; | ||
|
|
||
|
|
@@ -53,9 +53,9 @@ class Settings extends DefaultSettings<Props, State> { | |
| const hasChanges = !isEqual(this.state.initialData, this.state.formData); | ||
|
|
||
| const requiredFields = this.filterFields(this.state.fieldList, this.REQUIRED_FIELDS); | ||
| const selfHostedFields = this.filterFields( | ||
| this.state.fieldList, | ||
| this.SELF_HOSTED_FIELDS | ||
| ); | ||
|
|
||
| return ( | ||
|
|
@@ -68,19 +68,19 @@ class Settings extends DefaultSettings<Props, State> { | |
| </div> | ||
| )} | ||
| {this.renderFields(requiredFields)} | ||
| {selfHostedFields.length > 0 ? ( | ||
| <div className="control-group"> | ||
| <button | ||
| className="btn btn-default" | ||
| type="button" | ||
| onClick={this.toggleSelfHostedConfiguration} | ||
| > | ||
| Configure self-hosted | ||
| </button> | ||
| </div> | ||
| ) : null} | ||
| {this.state.showSelfHostedConfiguration | ||
| ? this.renderFields(selfHostedFields) | ||
| : null} | ||
| </Form> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,17 +67,17 @@ class SettingsIndex extends React.Component<Props> { | |
| render() { | ||
| const {organization} = this.props; | ||
| const user = ConfigStore.get('user'); | ||
| const isSelfHosted = ConfigStore.get('isSelfHosted'); | ||
|
|
||
| const organizationSettingsUrl = | ||
| (organization && `/settings/${organization.slug}/`) || ''; | ||
|
|
||
| const supportLinkProps = { | ||
| isSelfHosted, | ||
| href: LINKS.FORUM, | ||
| to: `${organizationSettingsUrl}support`, | ||
| }; | ||
| const supportText = isSelfHosted ? t('Community Forums') : t('Contact Support'); | ||
|
There was a problem hiding this comment. Actually I should update the forums link as well to keep up with getsentry/self-hosted#1151, but that's out of scope for this repo. |
||
|
|
||
| return ( | ||
| <SentryDocumentTitle | ||
|
|
@@ -340,7 +340,7 @@ const ExternalHomeLink = styled( | |
| `; | ||
|
|
||
| type SupportLinkProps<T extends boolean> = { | ||
| isSelfHosted: T; | ||
| href: string; | ||
| to: string; | ||
| isCentered?: boolean; | ||
|
|
@@ -350,12 +350,12 @@ type SupportLinkProps<T extends boolean> = { | |
|
|
||
| const SupportLinkComponent = <T extends boolean>({ | ||
| isCentered, | ||
| isSelfHosted, | ||
| href, | ||
| to, | ||
| ...props | ||
| }: SupportLinkProps<T>) => | ||
| isSelfHosted ? ( | ||
| <ExternalHomeLink isCentered={isCentered} href={href} {...props} /> | ||
| ) : ( | ||
| <HomeLink to={to} {...props} /> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the risky part. We need to make sure that
getsentry(and I guess whatever goofy third-party stuff exists out in the wild) doesn't break. As we can see,SENTRY_ONPREMISEdefaults to true, and probably nothing butgetsentryoverrides that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too important for this pr, but if we want to remove
SENTRY_ONPREMISEwe should also check with the single tenant team to see whether they set this variable differently as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call @wedamija. This came up at #30528 (comment) as well, and I've put single-tenant on the pile at getsentry/self-hosted#796 to make sure it isn't forgotten. Close one!