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

Fix issue when uploading to new buckets not in the standard region #303

Merged
merged 6 commits into from
Jun 10, 2014

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Jun 6, 2014

This PR implements the expect 100 continue support in order to address the case where you can't upload large files to newly created bucket in the non standard region. The fix consisted of 3 parts:

  • The HTTPConnection/Response subclasses that implements the actual expect 100 continue logic.
  • The plumbing to get it into urllib3's connection pools (which requests uses).
  • A handler to determine when to inject the Expect header into a request.

cc @danielgtaylor

jamesls added 5 commits June 5, 2014 13:59
This creates an HTTP (not HTTPS) subclass and just
copies the class dictionary of the HTTP method overrides
into the HTTPS subclass.  Added an integration test for this
as well.
def add_expect_header(operation, params, **kwargs):
if operation.http.get('method', '') not in ['PUT', 'POST']:
return
if operation.service.endpoint_prefix != 's3':
Copy link
Member

Choose a reason for hiding this comment

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

Since we use before-call.s3 for the event, is checking the endpoint prefix necessary here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's not necessary, I'll remove.

@danielgtaylor
Copy link
Member

Outside of my comment above, this looks good to me.

This can never happen because we subscribe to a specific
s3 event.  This was mentioned in code review feedback.
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.

2 participants