Navigation Menu

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

Accept responses with no etag header #3865

Merged
merged 3 commits into from Aug 1, 2020
Merged

Accept responses with no etag header #3865

merged 3 commits into from Aug 1, 2020

Conversation

indirect
Copy link
Member

If the server does not provide an etag header, treat all responses as a
cache miss. This should allow 3rd party gem servers to more easily
provide compact_index results. /cc @mperham

See also sidekiq/sidekiq#4158.

If the server does not provide an etag header, treat all responses as a
cache miss. This should allow 3rd party gem servers to more easily
provide compact_index results. /cc @mperham

See also sidekiq/sidekiq#4158.
@indirect
Copy link
Member Author

@deivid-rodriguez have you seen this kind of failure on windows before? I'm confused about why it passes everywhere but windows 😬

@deivid-rodriguez
Copy link
Member

Yes, I've seen those in the past in other places. I think it would happen randomly only on Windows when trying to rename a folder while being inside it.

@deivid-rodriguez
Copy link
Member

I have no idea why the changes in this PR make the failure happen, but since the spec failing is the one being touched by this PR, it should be related to these changes.

@deivid-rodriguez
Copy link
Member

I'll have a look at this tomorrow, see if I can figure it out.

@indirect
Copy link
Member Author

Oh no, I think I figured it out. The existing test hard-codes /tmp/localpath, which doesn't exist in Windows. How did this ever pass on Windows before?? 😬 Hopefully this will fix it...

@duckinator
Copy link
Member

This looks good to me. 👍

@duckinator duckinator merged commit 14e11e0 into master Aug 1, 2020
@duckinator duckinator deleted the allow-no-etag branch August 1, 2020 17:47
@fschwahn
Copy link

@indirect this patch is not in 2.2.0.rc.2 - with which version will this be released? This fixes a critical bug which makes it impossible to run bundle update for me (which incapacitates eg. dependabot). It's a bit hard to follow development now that the repositories are merged with rubygems (eg. no branches / no tags).

@deivid-rodriguez
Copy link
Member

I forgot to backport this, sorry. Will set it for the next release.

@fschwahn
Copy link

awesome, thanks!

@deivid-rodriguez
Copy link
Member

It's a bit hard to follow development now that the repositories are merged with rubygems (eg. no branches / no tags).

Stable branch for bundler-2.2 is that same as for rubygems-3.2 (the 3.2 branch), and tags are prefixed with bundler-. I know it's currently very weird, will get less weird as we improve things.

@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Oct 15, 2020

@indirect I wasn't sure of how to tag this PR. I went with bundler: minor improvement, but based on #3865 (comment) it could well be tagged as bundler: bug fix too? 🤔

@fschwahn
Copy link

Stable branch for bundler-2.2 is that same as for rubygems-3.2 (the 3.2 branch), and tags are prefixed with bundler-. I know it's currently very weird, will get less weird as we improve things.

Thanks, that helps. I didn't find the tags because github truncates the list, but searching works fine 👍

@indirect
Copy link
Member Author

@deivid-rodriguez yeah, I would categorize this as a bugfix. It was the intended behavior, and this was fixing the implementation to match what we intended. 👍

@deivid-rodriguez
Copy link
Member

Thanks @indirect!

deivid-rodriguez pushed a commit that referenced this pull request Dec 7, 2020
Accept responses with no etag header

(cherry picked from commit 14e11e0)
deivid-rodriguez pushed a commit that referenced this pull request Dec 7, 2020
Accept responses with no etag header

(cherry picked from commit 14e11e0)
deivid-rodriguez pushed a commit that referenced this pull request Dec 7, 2020
Accept responses with no etag header

(cherry picked from commit 14e11e0)
deivid-rodriguez pushed a commit that referenced this pull request Dec 7, 2020
Accept responses with no etag header

(cherry picked from commit 14e11e0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants