Consolidate env reading to single config object.#600
Merged
purple4reina merged 52 commits intomainfrom Jun 11, 2025
Merged
Conversation
fa21286 to
95cb0d6
Compare
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: |
Contributor
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.
37e4ccb to
6449589
Compare
6449589 to
2957287
Compare
purple4reina
commented
Jun 10, 2025
| 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 |
Contributor
Author
There was a problem hiding this comment.
These globals are not required because the affected objects are lists.
f869908 to
7bc8751
Compare
joeyzhao2018
approved these changes
Jun 11, 2025
Contributor
joeyzhao2018
left a comment
There was a problem hiding this comment.
LGTM. Thank you for doing this.
happynancee
approved these changes
Jun 11, 2025
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.
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