Skip to content

etags for index.json invalidation, test coverage#34641

Merged
alalazo merged 8 commits intospack:developfrom
haampie:feature/etag-and-buildcache-index-fetch-tests
Dec 21, 2022
Merged

etags for index.json invalidation, test coverage#34641
alalazo merged 8 commits intospack:developfrom
haampie:feature/etag-and-buildcache-index-fetch-tests

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Dec 21, 2022

Implement an alternative strategy to do index.json invalidation.

The current approach of pairs of index.json / index.json.hash is
problematic because it leads to races.

The standard solution for cache invalidation is etags, which are
supported by both http and s3 protocols, which allows one to do
conditional fetches.

This PR implements that for the http/https schemes. It should also work
for s3 schemes, but that requires other prs to be merged.

Also it improves unit tests for index.json fetches.

Implement an alternative strategy to do index.json invalidation.

The current approach of pairs of index.json / index.json.hash is
problematic because it leads to races.

The standard solution for cache invalidation is etags, which are
supported by both http and s3 protocols, which allows one to do
conditional fetches.

This PR implements that for the http/https schemes. It should also work
for s3 schemes, but that requires other prs to be merged.

Also it improves unit tests for index.json fetches.
@spackbot-app spackbot-app bot added binary-packages core PR affects Spack core functionality fetching tests General test capability(ies) utilities labels Dec 21, 2022
@haampie
Copy link
Copy Markdown
Member Author

haampie commented Dec 21, 2022

@blue42u the stuff you're overhauling has really bad test coverage, so I'm inclined to get this in first.

Copy link
Copy Markdown
Contributor

@blue42u blue42u left a comment

Choose a reason for hiding this comment

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

I'm more or less fine with landing this first. See behavioral concerns below.

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Dec 21, 2022

It would be nice if Gitlab CI would work here (and everywhere else too)

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Dec 21, 2022

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Dec 21, 2022

I've started that pipeline for you!

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Dec 21, 2022

@spackbot rebuild everything

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Dec 21, 2022

I've started that pipeline for you!

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Dec 21, 2022

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Dec 21, 2022

I've started that pipeline for you!

Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

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

@haampie and @blue42u agreed on this, so merging! Thanks!

@alalazo alalazo merged commit 4473d5d into spack:develop Dec 21, 2022
@haampie haampie deleted the feature/etag-and-buildcache-index-fetch-tests branch December 21, 2022 17:42
stephenmsachs pushed a commit to stephenmsachs/spack that referenced this pull request Jan 3, 2023
Implement an alternative strategy to do index.json invalidation.

The current approach of pairs of index.json / index.json.hash is
problematic because it leads to races.

The standard solution for cache invalidation is etags, which are
supported by both http and s3 protocols, which allows one to do
conditional fetches.

This PR implements that for the http/https schemes. It should also work
for s3 schemes, but that requires other prs to be merged.

Also it improves unit tests for index.json fetches.
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 24, 2023
Implement an alternative strategy to do index.json invalidation.

The current approach of pairs of index.json / index.json.hash is
problematic because it leads to races.

The standard solution for cache invalidation is etags, which are
supported by both http and s3 protocols, which allows one to do
conditional fetches.

This PR implements that for the http/https schemes. It should also work
for s3 schemes, but that requires other prs to be merged.

Also it improves unit tests for index.json fetches.
amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023
Implement an alternative strategy to do index.json invalidation.

The current approach of pairs of index.json / index.json.hash is
problematic because it leads to races.

The standard solution for cache invalidation is etags, which are
supported by both http and s3 protocols, which allows one to do
conditional fetches.

This PR implements that for the http/https schemes. It should also work
for s3 schemes, but that requires other prs to be merged.

Also it improves unit tests for index.json fetches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binary-packages core PR affects Spack core functionality fetching tests General test capability(ies) utilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants