-
Notifications
You must be signed in to change notification settings - Fork 596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate Hail import/export script [VS-605] #8002
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## ah_var_store #8002 +/- ##
================================================
Coverage ? 86.242%
Complexity ? 35202
================================================
Files ? 2173
Lines ? 165016
Branches ? 17792
================================================
Hits ? 142313
Misses ? 16377
Partials ? 6326 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious, why no last modified checks? Was it to keep the code simpler?
Mostly because I couldn't readily think of a scenario where I would actually want this to call cache, but I could easily imagine call caching leading to undesired clobbering of previously generated results. We can certainly revisit this decision if it turns out we're using the script in ways where we really would want call caching. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻 Only suggestion is to maybe change the comments explaining why the tasks are volatile from "there's no last modified check" to something like "we don't envision taking advantage of call caching at the moment".
No description provided.