Remove wrappers to ramble.config.config.get.. in workspace - #1752
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors workspace configuration retrieval by replacing class-specific getter methods with direct calls to ramble.config.get(), allowing the removal of several deprecated helper methods in the Workspace class. Feedback on these changes highlights a missing namespace import in google_batch.py that will cause a NameError, as well as multiple instances where using .copy() on configuration dictionaries creates a shallow copy, potentially leading to in-memory cache corruption. It is recommended to use copy.deepcopy() for these dictionaries.
Ramble Performance Test MetricsResults produced with commit: 17aff2e
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors workspace configuration access across the codebase by replacing Workspace instance helper methods (such as get_workspace_vars() and get_software_dict()) with direct calls to ramble.config.get(). This allows for the removal of several redundant getter methods in the Workspace class. The review feedback highlights two critical issues: a missing import of namespace in google_batch.py that will cause a NameError, and a potential configuration cache corruption in workspace.py due to mutating full_software_dict in-place without deep-copying it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1752 +/- ##
===========================================
- Coverage 94.07% 94.06% -0.01%
===========================================
Files 370 370
Lines 37399 37378 -21
===========================================
- Hits 35182 35161 -21
Misses 2217 2217 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
620295b to
dcfff90
Compare
dcfff90 to
17aff2e
Compare
No description provided.