Add support for more ENVs #3664
Conversation
| @@ -66,7 +66,7 @@ def get_internal_network(): | |||
|
|
|||
| # Instruct Sentry that this install intends to be run by a single organization | |||
| # and thus various UI optimizations should be enabled. | |||
| SENTRY_SINGLE_ORGANIZATION = True | |||
| SENTRY_SINGLE_ORGANIZATION = env("SENTRY_SINGLE_ORGANIZATION", "false").lower() in ("true") | |||
There was a problem hiding this comment.
| SENTRY_SINGLE_ORGANIZATION = env("SENTRY_SINGLE_ORGANIZATION", "false").lower() in ("true") | |
| SENTRY_SINGLE_ORGANIZATION = env("SENTRY_SINGLE_ORGANIZATION", "true").lower() in ("true") |
oops
There was a problem hiding this comment.
If they enable multi-organization, they need to enable another feature flag though. See https://github.com/getsentry/sentry/blob/89479bb784838445845564e0fba06b19190aa573/src/sentry/features/temporary.py#L41-L42
There was a problem hiding this comment.
I'd also make it accept 1, yes, y, and t too just to be comprehensive.
| "NAME": env("SENTRY_DB_NAME", "postgres"), | ||
| "USER": env("SENTRY_DB_USER", "postgres"), | ||
| "PASSWORD": env("SENTRY_DB_PASSWORD", ""), | ||
| "HOST": env("SENTRY_DB_HOST", "postgres"), | ||
| "PORT": env("SENTRY_DB_PORT", ""), |
There was a problem hiding this comment.
Do you mind renaming these to SENTRY_POSTGRES_DB_*? Since we have a few DBs..
| @@ -66,7 +66,7 @@ def get_internal_network(): | |||
|
|
|||
| # Instruct Sentry that this install intends to be run by a single organization | |||
| # and thus various UI optimizations should be enabled. | |||
| SENTRY_SINGLE_ORGANIZATION = True | |||
| SENTRY_SINGLE_ORGANIZATION = env("SENTRY_SINGLE_ORGANIZATION", "false").lower() in ("true") | |||
There was a problem hiding this comment.
If they enable multi-organization, they need to enable another feature flag though. See https://github.com/getsentry/sentry/blob/89479bb784838445845564e0fba06b19190aa573/src/sentry/features/temporary.py#L41-L42
| SENTRY_OPTIONS["mail.from"] = env('SENTRY_MAIL_FROM', f"sentry@{SENTRY_OPTIONS['mail.list-namespace']}") | ||
| SENTRY_OPTIONS["mail.port"] = env('SENTRY_MAIL_PORT', 465) | ||
| SENTRY_OPTIONS["mail.username"] = env('SENTRY_MAIL_USERNAME', '') | ||
| SENTRY_OPTIONS["mail.password"] = env('SENTRY_MAIL_PASSWORD', '') | ||
| SENTRY_OPTIONS["mail.use-tls"] = env('SENTRY_MAIL_USE_TLS', 'false').lower() in ('true') | ||
| SENTRY_OPTIONS["mail.use-ssl"] = env('SENTRY_MAIL_USE_SSL', 'false').lower() in ('true') |
There was a problem hiding this comment.
Shouldn't these defined on the config.yml file, instead of here? Or do you disable those on config.yml and define the mail configurations here instead?
| @@ -22,3 +22,14 @@ HEALTHCHECK_RETRIES=10 | |||
| POSTGRES_MAX_CONNECTIONS=100 | |||
| # Set SETUP_JS_SDK_ASSETS to 1 to enable the setup of JS SDK assets | |||
| # SETUP_JS_SDK_ASSETS=1 | |||
| # You can also override the default value for the assets URL | |||
There was a problem hiding this comment.
| # You can also override the default value for the assets URL | |
| # You can override the default value for the assets URL |
| # You can also override the default value for the assets URL | ||
| # JS_SDK_LOADER_DEFAULT_SDK_URL=https://sentry.example.com/js-sdk/%s/bundle%s.min.js | ||
|
|
||
| # To enabled the memcached cache store |
There was a problem hiding this comment.
| # To enabled the memcached cache store | |
| # Enable memcached cache store |
| # You can disable features | ||
| # SENTRY_DISABLE_FEATURES=organizations:sso-rippling,organizations:sso-saml2 | ||
|
|
||
| # Or enable features |
There was a problem hiding this comment.
Each variable and its comment should be used and defined regardless of other variables.
| # Or enable features | |
| # You can enable features |
| "OPTIONS": {"ignore_exc": True}, | ||
| # If you wish to use memcached, use ENABLE_MEMCACHED=true | ||
|
|
||
| if env("ENABLE_MEMCACHED", "false").lower() in ("true"): |
There was a problem hiding this comment.
This should also default to true, right?
There was a problem hiding this comment.
I really appreciate the effort but I fail to see how this improves things. Keeping most of the settings in the config files was intentional.
Can you provide a bit more context around why you want this change in and why specifically you want to control these from env variables? Feels like you are not using self-hosted through our default docker compose setup, is that right?
| SENTRY_WEB_HOST = env("SENTRY_WEB_HOST", "0.0.0.0") | ||
| SENTRY_WEB_PORT = env("SENTRY_WEB_PORT", 9000) |
There was a problem hiding this comment.
We should almost never be modifying these as they don't do what you expect with a docker-compose setup.
This is a backport of most of my changes to the example config
If I can have most if it merged, that would reduce the diff from the original file. And ensure better upgrades.
Feel free to ask to remove parts of this.
This will also help with #3662
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.