BentoML get improvement
#825
Conversation
|
Hello @jackyzha0, Thanks for updating this PR. There are currently no PEP 8 issues detected in this PR. Cheers! 🍻 Comment last updated at 2020-06-24 06:00:39 UTC |
bentoml/cli/bento.py
Outdated
| _print_bento_info(bento, output_type) | ||
|
|
||
|
|
||
| def _unpack_jq_like_string(bento_dict, jq): |
There was a problem hiding this comment.
nit renaming jq to json_query or something more descriptive.
bentoml/cli/bento.py
Outdated
| bento_dict = bento_dict[_t] | ||
| except KeyError: | ||
| _echo( | ||
| f'Key {_t} not found, ignoring', CLI_COLOR_ERROR, |
There was a problem hiding this comment.
We can make this error message more descriptive to better inform the users. Failed to query {json_query}. Key {_t} is missing.
Codecov Report
@@ Coverage Diff @@
## master #825 +/- ##
==========================================
- Coverage 56.41% 54.89% -1.53%
==========================================
Files 116 120 +4
Lines 8608 8996 +388
==========================================
+ Hits 4856 4938 +82
- Misses 3752 4058 +306
Continue to review full report at Codecov.
|
bentoml/cli/bento.py
Outdated
| _print_bento_info(bento, output_type) | ||
|
|
||
|
|
||
| def _unpack_jq_like_string(bento_dict, json_query): |
There was a problem hiding this comment.
jq actually supports many other syntax(https://stedolan.github.io/jq/tutorial/), I'd prefer not to expose this to the user if it does not fully support all the possible jq queries(e.g. the syntax for accessing array item).
Getting the location is a very common task and we don't want to force every user to install jq just for that. But for advanced users who can write jq query, I think installing jq wouldn't be a problem and we should probably just let them use command line jq with our json output.
bentoml/cli/bento.py
Outdated
| if output_type == 'table': | ||
| _print_bento_table(bentos) | ||
| elif output_type == 'wide': | ||
| _print_bento_table(bentos, wide=True) | ||
| else: | ||
| for bento in bentos: | ||
| if print_location: |
There was a problem hiding this comment.
Only printing a list of path seems not that useful to the user, currently the user can already use bentoml list -o wide or bentoml get IrisClassifier -o wide to view the list of saved bundle alongside with the saved location. I think it's ok to just ignore the --print-location argument in the case of listing and only apply it to get
* add print_location flag * add python like jq interp option * fix test import and linting issues * fix typo lmao * linter * fix more linting issues and address review comments * remove jq-like parsing stuff
Description
Adds
--print-locationflag tobentoml get <bento>:<version>Motivation and Context
bentoml getcommand #819.--print-locationHow Has This Been Tested?
Locally.
Types of changes
Components (if applicable)
Checklist:
./dev/format.shand./dev/lint.shscript have passed(instructions).