Skip to content
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

feat(sdk): add external_modules option to save_model #2895

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

bojiang
Copy link
Member

@bojiang bojiang commented Aug 12, 2022

TODO:

  • doc
  • tests

for #2785

@bojiang bojiang requested review from ssheng, parano and a team as code owners August 12, 2022 02:28
@bojiang bojiang requested review from sauyon and removed request for a team August 12, 2022 02:28
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #2895 (ddcb764) into main (ddcb764) will not change coverage.
The diff coverage is n/a.

❗ Current head ddcb764 differs from pull request most recent head 4b9b1ce. Consider uploading reports for the commit 4b9b1ce to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2895   +/-   ##
=======================================
  Coverage   70.54%   70.54%           
=======================================
  Files         104      104           
  Lines        9497     9497           
=======================================
  Hits         6700     6700           
  Misses       2797     2797           

ssheng
ssheng previously approved these changes Aug 21, 2022
aarnphm
aarnphm previously approved these changes Aug 22, 2022
@aarnphm
Copy link
Member

aarnphm commented Aug 22, 2022

@bojiang can you fetch main and run the test again?

@bojiang bojiang dismissed stale reviews from aarnphm and ssheng via 3bc278d August 25, 2022 02:15
@ssheng ssheng added this to the 1.0.4 milestone Aug 25, 2022
@bojiang
Copy link
Member Author

bojiang commented Aug 25, 2022

rebased to the main

aarnphm
aarnphm previously approved these changes Aug 25, 2022
@@ -78,5 +78,4 @@ USER bentoml

ENTRYPOINT [ "{{ bento__entrypoint }}" ]

CMD [ "bentoml", "serve", "{{ bento__path }}", "--production" ]
Copy link
Member

Choose a reason for hiding this comment

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

Why Do we remove this? I guess we will handle this from the entrypoint?

Copy link
Member Author

Choose a reason for hiding this comment

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

If both CMD and ENTRYPOINT exist, the CMD would be passed to ENTRYPOINT if no args provided

bentoml/serve.py Outdated
Comment on lines 513 to 514
from circus.sockets import CircusSocket # type: ignore
from circus.watcher import Watcher # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from circus.sockets import CircusSocket # type: ignore
from circus.watcher import Watcher # type: ignore
from circus.sockets import CircusSocket
from circus.watcher import Watcher

@ssheng ssheng merged commit d3d9cdb into bentoml:main Aug 26, 2022
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.

None yet

3 participants