Skip to content

Allow credentials instead of key/secret for the S3 store #282

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
Murderlon merged 19 commits into from
Nov 18, 2022
Merged

Allow credentials instead of key/secret for the S3 store #282

Murderlon merged 19 commits into from
Nov 18, 2022

Conversation

@phillip-causing
Copy link
Contributor

@phillip-causing phillip-causing commented Aug 10, 2022

No description provided.

mpcausing and others added 8 commits July 27, 2022 19:12
Murderlon
Murderlon reviewed Aug 10, 2022
View reviewed changes
Copy link
Member

@Murderlon Murderlon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks the PR!

Couple things, and we should also add documentation to let users know this is supported.

@Murderlon Murderlon changed the title Feature setting credentials s3 Allow credentials instead of key/secret for the S3 store Aug 10, 2022
Murderlon
Murderlon reviewed Aug 11, 2022
View reviewed changes
Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I would like the test this functionality too in some test before merging I think. I'm not sure yet though what it takes to write a test for this.

Murderlon
Murderlon reviewed Aug 15, 2022
View reviewed changes
Murderlon
Murderlon reviewed Aug 22, 2022
View reviewed changes
Copy link
Member

@Murderlon Murderlon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Credentials is not correct (it doesn't have httpOptions and such) and something went wrong with your copy paste of my docs suggestion I think because it has unfinished sentences:

The credentials config is directly passed into the AWS SDK so.)

Lastly, I don't think it's possible to write a test for this because tus-node-server would need to be inside a container?

After this, I think we're ready to merge :)

@Murderlon
Copy link
Member

Murderlon commented Oct 18, 2022

Thanks for all the changes! I'm holding this off for #309, which is now being worked on the 1.x branch.

@Murderlon Murderlon mentioned this pull request Oct 31, 2022
Open
2 tasks
@Murderlon Murderlon merged commit a14cae7 into tus:master Nov 18, 2022
Murderlon added a commit that referenced this pull request Nov 18, 2022
@Murderlon
Merge branch 'master' into 1.x
24de83b 
* master:
  0.9.0
  Allow `credentials` instead of key/secret for the S3 store (#282)
  Clarify example for Fastify (#311)
Murderlon added a commit that referenced this pull request Nov 18, 2022
@Murderlon
Merge branch '1.x' into s3-optimal-part-size
c96a88f 
* 1.x:
  0.9.0
  Allow `credentials` instead of key/secret for the S3 store (#282)
  Update engines.node version requirement
  Add the `Expiration` extension, implement it in `FileStore` (#320)
  Lock CI Node.js version to 19.0.1
  Clarify example for Fastify (#311)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Murderlon Murderlon Murderlon 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

@phillip-causing @Murderlon @mpcausing