Fix CI test data cache behavior - #626
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughAdds a Changes
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e184e8df51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| registry_path = Path("dascore/data_registry.txt") | ||
| print(f"DATA_REGISTRY_HASH={sha256(registry_path.read_bytes()).hexdigest()}") | ||
| PY | ||
| constants = runpy.run_path("dascore/constants.py") |
There was a problem hiding this comment.
Avoid importing full constants module in cache metadata step
Loading DATA_VERSION via runpy.run_path("dascore/constants.py") executes the entire constants module, which imports heavy runtime deps (pandas, numpy, and dascore) that are not guaranteed in environments that only need cache restore. In this repo, .github/actions/mamba-install-dascore/action.yml invokes this cache action even when install-package: false (used by run_min_dep_tests before pip install -e .[test]), so the new metadata step can fail with ModuleNotFoundError before tests start; reading DATA_VERSION without executing module imports would avoid breaking that workflow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/cache-test-data/action.yml:
- Line 22: The output description for the `cache-key` in action.yml is
incomplete — it omits the `cache-number` component; update the `description`
value for the `cache-key` output (and the identical occurrence at the other
location) to state that the generated cache key is derived from OS, data
version, registry hash, and cache-number so the doc matches the actual generated
key.
- Around line 12-15: Update the description for the action input named
"prime-on-miss" to match the actual implementation: state that it only applies
when mode is set to "restore-save" (not any restore mode). Locate the
"prime-on-miss" input entry in action.yml and change the description text to
explicitly mention "only used when mode='restore-save'" so users won't assume it
applies to all restore-related modes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f417a29-fb82-46da-bcee-69267f5438ba
📒 Files selected for processing (7)
.github/actions/cache-test-data/action.yml.github/workflows/get_coverage.yml.github/workflows/prime_test_data_cache.yml.github/workflows/profile.yml.github/workflows/run_min_dep_tests.yml.github/workflows/runtests.yml.github/workflows/test_doc_build.yml
💤 Files with no reviewable changes (1)
- .github/workflows/prime_test_data_cache.yml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #626 +/- ##
==========================================
+ Coverage 99.49% 99.93% +0.44%
==========================================
Files 135 135
Lines 11570 11570
==========================================
+ Hits 11511 11562 +51
+ Misses 59 8 -51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Testing
Summary by CodeRabbit