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

Support for specifying custom presign request date #649

Merged
merged 2 commits into from
Apr 30, 2018

Conversation

cheungpat
Copy link
Contributor

@cheungpat cheungpat commented Apr 12, 2018

This allows the request date used for presigning the request to be
configurable.

I want to make the request date configurable because I want to specify a custom
request date that is not datetime.utcnow(). For example, I want to make the
presign link to stay unchanged for a certain time period, which makes browser
caching easier.

I understand that this might not be what you guys want but I am going to try
my luck anyway :)

PS: minio-js has support for custom request date too, but that parameter is not exposed in exported function.

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

You might need to update docs/API.md with this new change as well.

@cheungpat
Copy link
Contributor Author

oh thanks for reminding! sorry it is my first time contributing here : )

@harshavardhana
Copy link
Member

No problem this is a nice change.. thanks for the PR

@vadmeste
Copy link
Member

@cheungpat thanks for this PR. Are you going to update docs/API.md as suggested by @harshavardhana ?

@cheungpat
Copy link
Contributor Author

@vadmeste Oh yes. Thanks for reminding!

cheungpat and others added 2 commits April 28, 2018 15:52
This allows the request date used for presigning the request to be
configurable.
@cheungpat
Copy link
Contributor Author

Updated and rebased. Thanks!

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM & tested

@harshavardhana harshavardhana merged commit 8078f82 into minio:master Apr 30, 2018
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.

3 participants