Skip to content

Comments

Fix #4690: Custom S3 Endpoint#5059

Merged
kcondon merged 14 commits intoIQSS:developfrom
poikilotherm:4690-custom-s3-endpoint
Oct 9, 2018
Merged

Fix #4690: Custom S3 Endpoint#5059
kcondon merged 14 commits intoIQSS:developfrom
poikilotherm:4690-custom-s3-endpoint

Conversation

@poikilotherm
Copy link
Contributor

@poikilotherm poikilotherm commented Sep 17, 2018

Please review and merge, thank you! 😄

Will solve #4690 by ...

Related Issues

Pull Request Checklist

@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch 3 times, most recently from 44bd2e8 to 97eaadc Compare September 20, 2018 14:43
@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch 2 times, most recently from f3a89c5 to cbddc5b Compare September 24, 2018 08:47
@poikilotherm poikilotherm changed the title WIP: Fixing 4690: Custom S3 Endpoint Fix #4690: Custom S3 Endpoint Sep 24, 2018
@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch from cbddc5b to ca0aa99 Compare September 24, 2018 08:48
@pdurbin pdurbin mentioned this pull request Sep 24, 2018
Copy link
Contributor

@matthew-a-dunlap matthew-a-dunlap left a comment

Choose a reason for hiding this comment

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

Overall this looks great! Standard aws functions work for me. I have a few thoughts and some other folks want to look over the docs in more depth.

@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch from 999de28 to 42d9469 Compare September 28, 2018 11:49
@poikilotherm
Copy link
Contributor Author

Attention: I just rebased onto latest development branch as #2940 has been fixed and #5071 has been merged.

@coveralls
Copy link

coveralls commented Sep 28, 2018

Coverage Status

Coverage increased (+0.05%) to 15.682% when pulling d9146b6 on poikilotherm:4690-custom-s3-endpoint into edff192 on IQSS:develop.

@pdurbin
Copy link
Member

pdurbin commented Sep 28, 2018

@poikilotherm can you please resolve the merge conflicts?

@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch 3 times, most recently from 08bd6cb to 060fbc8 Compare September 28, 2018 20:19
@poikilotherm
Copy link
Contributor Author

Sorry @dlmurphy, had to cherry-pick again because of changes. I added some more notes to the docs as requested by @pdurbin, maybe you can give this another look? Thx!

@dlmurphy
Copy link
Contributor

No problem, @poikilotherm. I just took a look at your recent edits to the docs and they look good to me. Thanks for all your work on this.

…aven property 'compilerArgument'. The property name was passed as argument string to the compiler, which of course failed. Adding an empty default fixes this.
…isible and adding a constructor to inject a mocked AWS S3 Client
@poikilotherm poikilotherm force-pushed the 4690-custom-s3-endpoint branch from eb2ae87 to b7b5d8a Compare October 9, 2018 13:59
@poikilotherm
Copy link
Contributor Author

@kcondon: I just pushed a freshly rebased feature branch. Thx for your proper job on the QA part! 😄

@kcondon kcondon merged commit f9f9be2 into IQSS:develop Oct 9, 2018
poikilotherm added a commit to poikilotherm/dataverse that referenced this pull request Nov 19, 2018
kcondon added a commit that referenced this pull request Nov 29, 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.

7 participants