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

Update BentoML example #1479

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Update BentoML example #1479

merged 1 commit into from
Mar 8, 2024

Conversation

naddeoa
Copy link
Contributor

@naddeoa naddeoa commented Mar 7, 2024

This updates the example for BentoML to no longer use atexit or the original rolling logger, and it uses the more common (in BentoML examples) function based service interface. It also highlights logging to multiple dataset ids within a single service.

The atexit only really works for toy local examples. BentoML prefers that you use its lifecycle management hooks to do startup and shutdown related logic. The old logger couldn't gracefully handle multiple dataset ids either, it required changing the default dataset id between logging calls.

@naddeoa naddeoa force-pushed the bentoml branch 5 times, most recently from a74bf54 to 25a7215 Compare March 7, 2024 23:50
jamie256
jamie256 previously approved these changes Mar 8, 2024
Copy link
Contributor

@jamie256 jamie256 left a comment

Choose a reason for hiding this comment

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

Looks good.

There is a reference to the whylogs[whylabs] extra which if that’s for pip install that extra is no longer required.

python/examples/integrations/bentoml/README.md Outdated Show resolved Hide resolved
This updates the example for BentoML to no longer use atexit or the
original rolling logger, and it uses the more common (in BentoML
examples) function based service interface. It also highlights logging
to multiple dataset ids within a single service.

The atexit only really works for toy local examples. BentoML prefers
that you use its lifecycle management hooks to do startup and shutdown
related logic. The old logger couldn't gracefully handle multiple
dataset ids either, it required changing the default dataset id between
logging calls.
@naddeoa
Copy link
Contributor Author

naddeoa commented Mar 8, 2024

@jamie256 updated to fix comments

Copy link
Contributor

@jamie256 jamie256 left a comment

Choose a reason for hiding this comment

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

LGTM!

@jamie256 jamie256 merged commit d2695da into mainline Mar 8, 2024
19 checks passed
@jamie256 jamie256 deleted the bentoml branch March 8, 2024 20:36
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

2 participants