Skip to content

new env config - #8

Open
trumbullm wants to merge 5 commits into
masterfrom
mt/env-var-configs
Open

new env config#8
trumbullm wants to merge 5 commits into
masterfrom
mt/env-var-configs

Conversation

@trumbullm

Copy link
Copy Markdown

No description provided.

@trumbullm
trumbullm requested a review from piehld August 28, 2026 15:46
@trumbullm

Copy link
Copy Markdown
Author

Most rcsb.workflow use a ConfigUtil object to pull in the exdb-loader-config.yaml. I want to be able to pass MONGO_DB_URI PDB_REPO_URL STASH_SERVER_URL and STASH_SERVER_FALLBACK_URL to these tasks. The simplest way with the fewest code changes seems to be via environment variable.

_getEnvConfigsDict() will go through every environment variable and pulls out those that start with CONFIG__ adding it to the self.__cD that all the workflow tasks use.

CONFIG__db__host=localhost becomes {"db": {"host": "localhost"}}

@rcsb rcsb deleted a comment from trumbullm Aug 31, 2026
Comment thread rcsb/utils/config/ConfigUtil.py Outdated
Comment on lines +726 to +748
def __getEnvConfigsDict(self):
"""Build a nested config dict from CONFIG__-prefixed environment variables.

``CONFIG__db__host=localhost`` becomes ``{"db": {"host": "localhost"}}``.

Returns:
dict: nested dictionary of configuration options
"""
config = {}
keys = []
for key, value in os.environ.items():
if not key.startswith("CONFIG__"):
continue
# to be logged
keys.append(key)
try:
key1, key2 = key.removeprefix("CONFIG__").split("__")
except ValueError:
logger.warning("Skipping %s", key)
continue
config.setdefault(key1, {})[key2] = value
logger.info("Found environment configs: %s", keys)
return config

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not a huge fan of this approach... It seems very hackish to rely on the specific naming structure of environmental variables. This could lead to confusion down the road (e.g., if someone were to see a weird envvar like CONFIG__db__host and think, "This seems unnecessarily long—let me clean it up"), as well as leaves open the chance of collisions with some other pre-existing envvar that happens to take on that naming structure.

Maybe with something more specific like EXDBCONFIG__..., then I'd feel better, but still it seems quite hackish.

I'd like to take a moment to think about some other ways we could possibly approach this, though...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm happy to go a different route. I'm having trouble finding a nice way to do it though. We could change the code so its passed by cli for every task but these values are generally passed down the chain individually, instead of a dict. etl_tree_node_lists passes over to TreeNodeListWorker passes to DocumentLoader passes to Connection() which then pulls it in from self.__cfgOb.

This is a fine course of action but its just a lot of code changes across a lot of repos.

@trumbullm
trumbullm requested a review from piehld September 1, 2026 17:47
@trumbullm

Copy link
Copy Markdown
Author

@piehld CONFIG_SUPPORT_TOKEN is pulled in directly from the env, but its a good idea to just check the value to see if its an env var. Now in this PR when a value is requested it will check if it starts with ENV_ and if it does it will go get it from the environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants