8000 Remove "millisecond" suffix from getSignedUrl() function by chr4 · Pull Request #5302 · outline/outline · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove "millisecond" suffix from getSignedUrl() function #5302

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
Merged
merged 1 commit into from
May 5, 2023
Merged

Remove "millisecond" suffix from getSignedUrl() function #5302

merged 1 commit into from
May 5, 2023

Conversation

chr4
Copy link
Contributor
@chr4 chr4 commented May 4, 2023

The parameter actually expects seconds and not milliseconds, according to S3 documentation:

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html

This was confusing me when debugging an issue with S3.

@auto-assign auto-assign bot requested a review from tommoor May 4, 2023 10:25
@CLAassistant
Copy link
CLAassistant commented May 4, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@tommoor
Copy link
Member
tommoor commented May 4, 2023

Yea, definitely a mistake – the types/docs for aws-sdk are any here, so not a surprise. Mind updating

outline/server/models/helpers/DocumentHelper.tsx

Line 301 in db73879

* @param expiresIn The time that signed urls should expire in (ms)
too?

@chr4
Remove "millisecond" suffix from getSignedUrl() function
eaa58bc 
The parameter actually expects seconds and not milliseconds, according
to S3 documentation:

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html
@chr4
Copy link
Contributor Author
chr4 commented May 4, 2023

Also adjusted the mentioned comment!

@chr4 chr4 force-pushed the chore/rm-s3-ms branch from 65ce528 to eaa58bc Compare May 4, 2023 17:17
@tommoor tommoor merged commit d096ba4 into outline:main May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tommoor tommoor tommoor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chr4 @CLAassistant @tommoor
0