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

Adding optional content type argument to generate signed URI method #1447

Merged
merged 4 commits into from Feb 17, 2016
Merged

Adding optional content type argument to generate signed URI method #1447

merged 4 commits into from Feb 17, 2016

Conversation

ghost
Copy link

@ghost ghost commented Feb 12, 2016

Fix for #1429. Using default content type as application/octet-stream.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 12, 2016
@ghost
Copy link
Author

ghost commented Feb 12, 2016

Appveyor build failing is covered on #1434.

dhermes
dhermes reviewed Feb 12, 2016
View reviewed changes
gcloud/storage/blob.py
response_type=None, response_disposition=None,
generation=None):
def generate_signed_url(self, expiration, method='GET', client=None,
content_type="application/octet-stream",

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Feb 13, 2016
@ghost
Copy link
Author

ghost commented Feb 13, 2016

@googlebot Check CLA again, please.

dhermes
dhermes reviewed Feb 13, 2016
View reviewed changes
gcloud/storage/blob.py
client=None, credentials=None,
response_type=None, response_disposition=None,
generation=None):
content_type='application/octet-stream',

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Feb 13, 2016
@tseaver
Copy link
Contributor

tseaver commented Feb 15, 2016

@cyluss can you please add a unit test to test_blob which exercises the explicit content_type argument, and asserts that it is passed through?

@ghost
Copy link
Author

ghost commented Feb 16, 2016

@tseaver Here you are. I appreciate for your feedback.

@tseaver
Copy link
Contributor

tseaver commented Feb 16, 2016

LGTM.

@dhermes do I recall correctly that you meant to be turning off Appveyor for PRs?

@dhermes
Copy link
Contributor

dhermes commented Feb 16, 2016

@tseaver I have set "Branches to build: Only branches specified below" to master, and it doesn't seem to matter.

@cyluss
Adding test that covers explicit content type argument
791e1e7 
- Testing with plain name (+2 squashed commits)
Squashed commits:
[d3ffb14] Keep code DRY
[73c285c] Adding test that covers explicit content type argument
tseaver added a commit that referenced this pull request Feb 17, 2016
@tseaver
Merge pull request #1447 from cyluss/fix-1429
dc26221 
Adding optional content type argument to generate signed URI method
@tseaver tseaver merged commit dc26221 into googleapis:master Feb 17, 2016
@tseaver
Copy link
Contributor

tseaver commented Feb 17, 2016

Thanks very much, @cyluss!

@ghost
Copy link
Author

ghost commented Feb 17, 2016

@tseaver Your team gave me great experience. Thank you.

@ghost ghost deleted the fix-1429 branch February 17, 2016 13:52
@dhermes dhermes mentioned this pull request Feb 17, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
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.
4 participants
@googlebot @tseaver @dhermes @cyluss