refactor: replace get_env_vars() tuple with frozen EvergreenConfig dataclass#525
Merged
refactor: replace get_env_vars() tuple with frozen EvergreenConfig dataclass#525
Conversation
Collaborator
Author
|
|
…enConfig dataclass ## What Replace the 30-element tuple returned by get_env_vars() with a frozen dataclass EvergreenConfig. Updated evergreen.py main() to use config.attribute access instead of tuple destructuring, and converted all 20 expected_result tuples in test_env.py to use named EvergreenConfig construction. Includes the ghe_api_url field from PR #524. ## Why The 30-element tuple was fragile — adding/removing fields required counting positions across 20+ tests, and positional access like result[8] was unreadable. Named attribute access eliminates these problems and makes future field additions trivial. ## Notes - This is a pure refactor with no behavior change. All 173 tests pass with identical coverage. - The dataclass uses frozen=True for immutability, matching the previous tuple's semantics. - pylint disable for too-many-instance-attributes is necessary since the config genuinely has 30 fields. - Rebased on main after PR #524 merge to include ghe_api_url field. Signed-off-by: jmeridth <jmeridth@gmail.com>
jmeridth
force-pushed
the
jm_refactor_env_vars_to_dataclass
branch
from
c0ea814 to
40d1f30
Compare
## What Changed group_dependencies, enable_security_updates, and update_existing from bool | None to bool in the EvergreenConfig dataclass. ## Why get_bool_env_var() always returns bool, never None. The overly permissive annotations were carried over from the old tuple type hints. Signed-off-by: jmeridth <jmeridth@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors environment configuration handling by replacing the large positional tuple returned by get_env_vars() with an immutable EvergreenConfig dataclass, improving readability and reducing brittleness across the action and tests.
Changes:
- Introduce
EvergreenConfig(frozen=True) and updateenv.get_env_vars()to return it. - Update
evergreen.pyto useconfig.<field>access instead of tuple destructuring. - Update
test_env.pyexpected values to constructEvergreenConfig(...)and replace tuple indexing with attribute access.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
env.py |
Adds EvergreenConfig dataclass and changes get_env_vars() to return it. |
evergreen.py |
Switches main flow from tuple unpacking to dataclass attribute access. |
test_env.py |
Converts tuple-based expected results and indexing assertions to EvergreenConfig usage. |
## What Changed search_query from str | None to str, and repo_specific_exemptions from bare dict to dict[str, list[str]] in EvergreenConfig. ## Why search_query always comes from os.getenv(..., "").strip() which never returns None. repo_specific_exemptions is built by parse_repo_specific_exemptions() which always returns dict[str, list[str]]. The tighter annotations improve IDE hints and mypy coverage. Signed-off-by: jmeridth <jmeridth@gmail.com>
zkoppert
approved these changes
Mar 30, 2026
Collaborator
zkoppert
left a comment
zkoppert
left a comment
There was a problem hiding this comment.
Looks great! I was initially wary of the type narrowing thinking that the change might cause a difference in conditional evaluations but looking through each one, it appears they were never a None value anyways. 👍🏻
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.
Pull Request
Fixes #527
Proposed Changes
What
Replace the 30-element tuple returned by
get_env_vars()with afrozen=TrueEvergreenConfigdataclass. Updatedevergreen.pymain()to useconfig.attributeaccess instead of tuple destructuring, and converted all 20expected_resulttuples intest_env.pyto use namedEvergreenConfigconstruction.Why
The 30-element tuple was fragile — adding or removing fields required counting positions across 20+ tests, and positional access like
result[8]was unreadable. Named attribute access eliminates these problems and makes future field additions trivial (just add a field with a default value).Notes
frozen=Truefor immutability, matching the previous tuple's semantics.pylint: disable=too-many-instance-attributesis necessary since the config genuinely has 30 fields. This is inherent to the problem domain, not a design issue.main()function inevergreen.pyispragma: no cover, so switching from destructuring toconfig.attraccess is verified by the type checker (mypy) rather than unit tests.Testing
test_env.pyconverted to namedEvergreenConfig(...)construction — verifies the dataclass fields match the function's return values exactly.test_env.pyunchanged and passing.Readiness Checklist
Author/Contributor
make lintand fix any issues that you have introducedmake testand ensure you have test coverage for the lines you are introducing