Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

fix: calling the bentoml's track() with incorrect CliEvent #176

Merged
merged 5 commits into from
Jul 20, 2022

Conversation

jjmachan
Copy link
Contributor

@jjmachan jjmachan commented Jul 7, 2022

Description

Curently bentoctl CliEvent is not defined with attrs and hence does not get serialized completely
when calling track() function in bentoml.
closes:

@jjmachan jjmachan requested a review from aarnphm July 7, 2022 19:42
# These are internal apis. We will need to make sure update these when BentoML changes.
from bentoml._internal.utils.analytics.schemas import EventMeta
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @ssheng another point we need to consider when discussing about API exposure post 1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a bentoml_util project will be needed for common libraries within our projects.

@codecov
Copy link

codecov bot commented Jul 7, 2022

Codecov Report

Merging #176 (9d2d9e2) into main (4d50cd7) will decrease coverage by 0.97%.
The diff coverage is 68.18%.

@@            Coverage Diff             @@
##             main     #176      +/-   ##
==========================================
- Coverage   60.27%   59.29%   -0.98%     
==========================================
  Files          24       24              
  Lines        1110     1113       +3     
==========================================
- Hits          669      660       -9     
- Misses        441      453      +12     
Flag Coverage Δ
unit-tests 59.29% <68.18%> (-0.98%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
bentoctl/cli/utils.py 61.76% <0.00%> (-11.77%) ⬇️
bentoctl/utils/usage_stats.py 78.26% <88.23%> (-16.74%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

initial readthrough lgtm.

@jjmachan jjmachan marked this pull request as ready for review July 11, 2022 05:31
@jjmachan jjmachan requested a review from aarnphm July 20, 2022 04:33
@jjmachan jjmachan merged commit f3673bc into bentoml:main Jul 20, 2022
@jjmachan jjmachan deleted the tracking branch July 20, 2022 05:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants