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.

Sign up for GitHub

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

Jump to bottom

fix: fix bugs in "det model describe" #1012

Merged
merged 1 commit into from
Aug 4, 2020
Merged

fix: fix bugs in "det model describe" #1012

merged 1 commit into from
Aug 4, 2020

Conversation

neilconway
Copy link
Contributor

@neilconway neilconway commented Aug 4, 2020 •
edited by jira bot
Loading

Description

  • "det model describe" failed for a model with no versions
  • "det model describe --version k" ignored the "--version" argument

Test Plan

Manually ran the CLI before/after these changes.

Commentary (optional)

@cla-bot cla-bot bot added the cla-signed label Aug 4, 2020
@neilconway
Copy link
Contributor Author

BTW, I noticed that det model describe --json omits information about model versions (whereas model versions are displayed in the non-JSON output). Not sure how much we care about that.

Also you could argue that using version = 0 the denote "fetch the latest version" is a bit confusing and using None would be cleaner, although I don't feel too strongly about it. If you want to make this change lmk and I can do it.

@neilconway
fix: fix bugs in "det model describe"
de405c3 
* "det model describe" failed for a model with no versions
* "det model describe --version k" ignored the "--version" argument
@neilconway neilconway force-pushed the cli-model-describe-no-versions branch from 986581d to de405c3 Compare August 4, 2020 21:26
sidneyw
sidneyw approved these changes Aug 4, 2020
View reviewed changes
Copy link
Contributor

@sidneyw sidneyw left a comment

Choose a reason for hiding this comment

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

Nice catch. This looks good to me. We'll have to make sure this behavior is the same in the autogenerated client when we move to it.

@sidneyw sidneyw assigned neilconway and unassigned sidneyw Aug 4, 2020
@neilconway neilconway merged commit c8e63dd into determined-ai:master Aug 4, 2020
@neilconway neilconway deleted the cli-model-describe-no-versions branch August 9, 2020 16:49
@dannysauer dannysauer added this to the 0.12.13 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sidneyw sidneyw sidneyw approved these changes

@aaron276h aaron276h Awaiting requested review from aaron276h

Assignees

@neilconway neilconway

Labels
cla-signed
Projects
None yet
Milestone
0.12.13
Development

Successfully merging this pull request may close these issues.
3 participants
@neilconway @sidneyw @dannysauer