enable smooth environment variable naming transition#311
Merged
Conversation
| @@ -130,7 +131,7 @@ def start(ctx, disable_collection, development, pidfile, port): | |||
| processes = start_celery_worker_processes((core_worker_count, secondary_worker_count, facade_worker_count), disable_collection) | |||
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0621: Redefining name 'processes' from outer scope (line 396) (redefined-outer-name)
| @@ -15,6 +15,7 @@ | |||
| import requests | |||
| from redis.exceptions import ConnectionError as RedisConnectionError | |||
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused ConnectionError imported from redis.exceptions as RedisConnectionError (unused-import)
| @@ -31,7 +32,7 @@ | |||
|
|
|||
| from keyman.KeyClient import KeyClient, KeyPublisher | |||
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused KeyClient imported from keyman.KeyClient (unused-import)
shlokgilda
requested changes
May 10, 2026
shlokgilda
left a comment
Contributor
There was a problem hiding this comment.
Added some inline comments
SystemEnv class to unify environment variable use and enable smooth transition8041729 to
1f89a32
Compare
This was referenced Jun 7, 2026
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…ode slowly over time Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…autoimport locations Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
so it doesn't import CollectOSS Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
github keys being put in wrong places Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…tential completion Temporary workaround until we have a faster DB initialization Signed-off-by: Adrian Edwards <adredwar@redhat.com>
e30329f to
c049811
Compare
this should hopefully take care of everything up front like the original scripts did, without creating issues related to the cursed import situation Signed-off-by: Adrian Edwards <adredwar@redhat.com>
| @@ -130,7 +132,7 @@ def start(ctx, disable_collection, development, pidfile, port): | |||
| processes = start_celery_worker_processes((core_worker_count, secondary_worker_count, facade_worker_count), disable_collection) | |||
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0621: Redefining name 'processes' from outer scope (line 397) (redefined-outer-name)
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…plication Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…re CollectOSS starts. This avoids some race conditions with the startup process that could create issues, especially on first initialization and especially in CI where we are still replaying migrations Co-Authored-By: guptapratykshh <pratykshgupta9999@gmail.com> Co-Authored-By: Sukuna0007Abhi <appsonly310@gmail.com> Signed-off-by: Adrian Edwards <adredwar@redhat.com>
9e2a164 to
bba22af
Compare
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a new
SystemEnvclass that lives alongsideSystemConfigand aims to centralize/unify all environment variable access.The primary purpose of this is to slightly decouple the requested environment variable name from the name where the value is actually pulled from. For example, even if all in-code references use
COLLECTOSS_DBfor the database connection string, this layer will allow a values to be read fromAUGUR_DBas a fallback ifCOLLECTOSS_DB(preferred) is not set.This class is also set up to print a deprecation warning/notice whenever the
AUGURvariant of an environment variable is used as a fallback. The message makes it clear that this fallback behavior is only temporary and may not be around forever (although I don't see this code being removed anytime in the foreseeable future)This PR includes a unit tests for this new functionality and works as intended, including edge cases such as when the key the code requests doesn't use a known prefix (
os.getenv("CONFIG_DATADIR"))It also includes a special case function
get_bool()because there were ~3 different places in the code where a small python one-liner (that i wrote) had been copied around for convenience, and this was a good opportunity to refactor that to prevent those three versions from ever drifting in implementation from each other. There are certainly more places this logic can be reused as well as we unify the way all env vars are processed.Environment variables handled via Click (the CLI library) are addressed too. the click automatic prefix has been updated and the ~5 lines in 1 file where we were explicitly setting the variables are now updated to include both variants.
This also addresses the init, config, and entrypoint bash scripts uses at startup by replacing them with python equivalents (and enabling them to utilize the new environment variable switcher as well). This effectively means the earliest phases of the startup sequence in CollectOSS have been cleaned up somewhat
This PR resolves one of the large items that needs to be done to complete #279 for the next release.
Notes for Reviewers
This change renames all code references to use the new
COLLECTOSS_prefix, with theSystemEnvadapting these accesses to be able to read fromAUGUR_variables ifCOLLECTOSS_ones aren't present.This PR works in the sense that CollectOSS is able to start up with this change present with the default env
Signed commits