Consolidate env reading to single config object. #600
Merged
Conversation
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
purple4reina
force-pushed
the
rey.abolofia/config-object
branch
7 times, most recently
from
fa21286 to
95cb0d6
Compare
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| return val.lower() == "true" or val == "1" | ||
|
|
||
|
|
||
| class Config: |
There was a problem hiding this comment.
⚪ Code Quality Violation
Class Config should have an init method (...read more)
Ensure that a class has an __init__ method. This check is bypassed when the class is a data class (annotated with @dataclass).
purple4reina
force-pushed
the
rey.abolofia/config-object
branch
3 times, most recently
from
7d2950e to
dd6b1d2
Compare
debug_encoding.py
Outdated
| # int. let's special case that here, because it's sure to happen in | ||
| # customer code. | ||
| err = d.get("error") | ||
| if err and type(err) == bool: |
There was a problem hiding this comment.
⚪ Code Quality Violation
use isinstance() instead of type() (...read more)
Using type checks if an object o is exactly a given type. In contrast, isinstance checks if o is a given type, or any subclass of the given type, thus supporting inheritance. In addition,isinstance is generally faster than type.
purple4reina
force-pushed
the
rey.abolofia/config-object
branch
5 times, most recently
from
37e4ccb to
6449589
Compare
purple4reina
force-pushed
the
rey.abolofia/config-object
branch
from
6449589 to
2957287
Compare
purple4reina
commented
Jun 10, 2025
| @@ -102,7 +101,6 @@ def pop_node(module_name): | |||
| end_time_ns = time.time_ns() | |||
| node.end_time_ns = end_time_ns | |||
| if not import_stack: # import_stack empty, a root node has been found | |||
| global root_nodes | |||
There was a problem hiding this comment.
These globals are not required because the affected objects are lists.
purple4reina
force-pushed
the
rey.abolofia/config-object
branch
from
f869908 to
7bc8751
Compare
| return [val.strip() for val in val.split(",") if val.strip()] | ||
|
|
||
|
|
||
| class Config: |
There was a problem hiding this comment.
⚪ Code Quality Violation
Class Config should have an init method (...read more)
Ensure that a class has an __init__ method. This check is bypassed when the class is a data class (annotated with @dataclass).
joeyzhao2018
approved these changes
Jun 11, 2025
happynancee
approved these changes
Jun 11, 2025
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.
What does this PR do?
Consolidate all config and (most) env var reading into a single
configobject.Furthermore, the environment is now only read when it's needed. The config object will only resolve a config setting when it is accessed. Meaning, we'll be reading a ton fewer environment variables during initialization which will lead to faster cold starts.
Motivation
Tests are currently failing on #598 with a circular import error. Consolidating all config reading to a single location will make fixing that much much much easier.
Testing Guidelines
Additional Notes
Note that for all the boolean like env vars, it is now possible to set the environment value to
"1"to configure for aTrueevaluation. The customer can then use"0"or really anything other than"true"or"1"to get aFalseevaluation.Types of Changes
Check all that apply