Get rid of Ramble's dependency on spack_json#1648
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the vendored spack.util.spack_json dependency with a new internal ramble.util.json_util module across the codebase, updating all references, adding unit tests, and enforcing this change via Ruff in pyproject.toml. Feedback suggests explicitly typing dump_args as Dict[str, Any] in json_util.py to improve type safety and remove the need for # type: ignore comments.
Instead, implement thin helpers under ramble.util.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request replaces the dependency on spack.util.spack_json with a new internal utility ramble.util.json_util across the codebase, enforcing this restriction via pyproject.toml. However, the new utility lacks a custom serialization handler for types like set, Enum, and objects with to_dict methods, which were previously supported by spack_json. This regression could lead to runtime serialization errors and non-deterministic hashing. It is recommended to implement a custom default handler in json_util and add corresponding unit tests to verify correct behavior.
Ramble Performance Test MetricsResults produced with commit: fbe9cb4
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1648 +/- ##
========================================
Coverage 93.30% 93.30%
========================================
Files 352 354 +2
Lines 34223 34256 +33
========================================
+ Hits 31931 31964 +33
Misses 2292 2292 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Instead, implement thin helpers under ramble.util.