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.

Sign up for GitHub

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

Jump to bottom

fix: exclude 308s from httplib2 redirect codes list #813

Merged
merged 6 commits into from Mar 11, 2020
Merged

fix: exclude 308s from httplib2 redirect codes list #813

merged 6 commits into from Mar 11, 2020

Conversation

busunkim96
Copy link
Contributor

Closes #803

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 24, 2020
@busunkim96 busunkim96 requested a review from a team January 24, 2020 17:55
plamut
plamut suggested changes Jan 30, 2020
View reviewed changes
Copy link

@plamut plamut left a comment •
edited

Choose a reason for hiding this comment

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

The fix looks good based on the discussion in the issue description, but let's also cover it with a test.

Not sure why Kokoro failed, though, its log is empty, or maybe just I cannot see it.

googleapiclient/http.py Outdated Show resolved Hide resolved
@renchap renchap mentioned this pull request Feb 4, 2020
Open
@temoto temoto mentioned this pull request Mar 4, 2020
Closed
@busunkim96 busunkim96 self-assigned this Mar 6, 2020
@busunkim96 busunkim96 requested a review from plamut March 6, 2020 18:03
plamut
plamut reviewed Mar 11, 2020
View reviewed changes
Copy link

@plamut plamut left a comment

Choose a reason for hiding this comment

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

The test case looks good, thanks for adding it. Kokoro configs are probably fine, too. :)

I added a minor suggestion to remove some unnecessary clutter from the code.

googleapiclient/http.py Outdated Show resolved Hide resolved
@busunkim96 @plamut
fix: remove 308 directly from redirect_codes
ca612fa 
Co-Authored-By: Peter Lamut <plamut@users.noreply.github.com>
@busunkim96 busunkim96 requested a review from plamut March 11, 2020 18:26
plamut
plamut approved these changes Mar 11, 2020
View reviewed changes
Copy link

@plamut plamut left a comment

Choose a reason for hiding this comment

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

LGTM

@busunkim96 busunkim96 merged commit b3b773f into googleapis:master Mar 11, 2020
@vorpal-buildbot vorpal-buildbot mentioned this pull request Mar 11, 2020
Merged
This was referenced May 11, 2020
Closed
Merged
@samccauley samccauley mentioned this pull request Jun 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plamut plamut plamut approved these changes

Assignees

@busunkim96 busunkim96

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

httplib2 v0.16.0 breaks the library
3 participants
@busunkim96 @plamut @googlebot