-
Notifications
You must be signed in to change notification settings - Fork 2
Feature to fetch temporary object store credentials #46
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
Conversation
joyceyuu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good overall. If you can answer my few questions, I am happy to approve it
| out = combine_clks_blocks(clk_json, blocks) | ||
| with open(clk_json, 'rb') as encodings: | ||
| out = combine_clks_blocks(encodings, blocks) | ||
| response = rest_client.project_upload_clks(project, apikey, out) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we don't upload to minio as well like the no blocking case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the initial plan we will only support binary encodings being uploaded to the object store. Eventually a binary format for the blocks will also be implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay
joyceyuu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good to me. Just few tests are currently failing. Maybe merge into master after fixing the tests
Codecov Report
@@ Coverage Diff @@
## master #46 +/- ##
==========================================
+ Coverage 91.44% 94.67% +3.22%
==========================================
Files 3 3
Lines 456 488 +32
==========================================
+ Hits 417 462 +45
+ Misses 39 26 -13 |
This modifies the cli client to retrieve object store credentials during upload.
For testing purposes it uploads the data twice - once to the existing api, and once to the object store.