Skip to content
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

Add env var to disable cache #2091

Merged
merged 2 commits into from
Mar 12, 2021
Merged

Add env var to disable cache #2091

merged 2 commits into from
Mar 12, 2021

Conversation

sir-sigurd
Copy link
Member

@sir-sigurd sir-sigurd commented Mar 5, 2021

Description

TODO

  • Unit tests
  • add unit test for PackageEntry.get_cached_path()
  • Documentation
  • Changelog entry

@codecov
Copy link

codecov bot commented Mar 5, 2021

Codecov Report

Merging #2091 (7a7cee6) into master (9664258) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2091      +/-   ##
==========================================
+ Coverage   45.83%   45.94%   +0.10%     
==========================================
  Files         420      420              
  Lines       19915    19954      +39     
  Branches     2306     2306              
==========================================
+ Hits         9129     9168      +39     
  Misses       9796     9796              
  Partials      990      990              
Flag Coverage Δ
api-python 89.54% <100.00%> (+0.07%) ⬆️
catalog 15.72% <ø> (ø)
lambda 92.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/python/quilt3/packages.py 92.60% <100.00%> (+0.11%) ⬆️
api/python/quilt3/util.py 83.44% <100.00%> (+0.16%) ⬆️
api/python/tests/integration/test_packages.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9664258...7a7cee6. Read the comment docs.

@sir-sigurd sir-sigurd force-pushed the disable-cache branch 5 times, most recently from 3336358 to fbcc915 Compare March 9, 2021 15:54
@sir-sigurd sir-sigurd requested a review from akarve March 9, 2021 16:03
@akarve akarve requested a review from dimaryaz March 9, 2021 18:09
@sir-sigurd sir-sigurd force-pushed the disable-cache branch 3 times, most recently from d690986 to 803a7c9 Compare March 10, 2021 07:36
# opened for concurrent access.
with tempfile.NamedTemporaryFile(delete=False) as tmp_file:
local_pkg_manifest = tmp_file.name
stack.callback(os.unlink, local_pkg_manifest)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little bit over-engineered... Ideally, we'd just download the manifest and immediately deserialize it, without even writing to a temporary file - or create a temporary file, but use its file descriptor instead of opening it again. But I guess that's not supported right now.

Fine for now, but would be nice to clean up some day.

@sir-sigurd sir-sigurd merged commit 46fd071 into master Mar 12, 2021
@sir-sigurd sir-sigurd deleted the disable-cache branch March 12, 2021 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants