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
Implement feature uploading bento repo to S3. #356
Conversation
Codecov Report
@@ Coverage Diff @@
## master #356 +/- ##
==========================================
- Coverage 53.67% 53.59% -0.09%
==========================================
Files 78 78
Lines 5246 5269 +23
==========================================
+ Hits 2816 2824 +8
- Misses 2430 2445 +15
Continue to review full report at Codecov.
|
Hello @leonsim, Thanks for updating this PR. There are currently no PEP 8 issues detected in this PR. Cheers! 🍻 Comment last updated at 2019-10-26 04:50:32 UTC |
8a61b5a
to
a89fc0c
Compare
@@ -122,3 +122,6 @@ built-docs | |||
|
|||
# MacOS X | |||
.DS_Store | |||
|
|||
# sqlite | |||
storage.db |
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.
this should not be created in the repo directory right? curious why adding this
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.
if I install BentoML by pip install -e .
then this file will be generated when I run a service.save() command.
|
||
# Return URI to saved bento in repository storage | ||
return response.uri.uri | ||
elif response.uri.type == BentoUri.S3: | ||
with tempfile.TemporaryDirectory() as tmpdir: | ||
|
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.
add something like update_bento_upload_progress(... , InProgress, progress = 0)
here? since the upload may take a while
tar.add(tmpdir, arcname=bento_service.name) | ||
fileobj.seek(0, 0) | ||
|
||
files = {'file': ('dummy', fileobj)} |
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.
could you add a comment about "dummy" here?
Thanks for updating the PR @leonsim , merging now! |
(Thanks for sending a pull request! Please make sure to read the contribution guidelines, then fill out the blanks below.)
What changes were proposed in this pull request?
Does this close any currently open issues?
How was this patch tested?