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

fix(metrics): get command relax validation to allow any --metric #334

Merged
merged 3 commits into from
Nov 23, 2020

Conversation

robghchen
Copy link
Contributor

@robghchen robghchen commented Nov 20, 2020

Currently we're only allowing get metrics for this hard coded list of options cpuPercentage, memoryUsage, gpuMemoryFree, gpuMemoryUsed, gpuPowerDraw, gpuTemp, gpuUtilization, gpuMemoryUtilization. if user inputs anything other than those options, the cli will raise an error. we want to relax this validation to support custom metrics

We also want to show known metrics in the --help text

QA plan:

  1. Try getting custom metric for deployments, run command gradient deployments metrics get --id desmvfs1gxebznb --metric iamcustom, should return null instead of showing an error
  2. Run command gradient deployments metrics --help, check if the get command help text now mentions cpuPercentage, memoryUsage, gpuMemoryFree, gpuMemoryUsed, gpuPowerDraw, gpuTemp, gpuUtilization, gpuMemoryUtilization
  3. Repeat 1 & 2 for jobs, experiments, and notebooks

@linear
Copy link

linear bot commented Nov 20, 2020

CR2-50 Show custom metrics options in gradient-cli get command

Research: https://linear.app/paperspace/issue/CR2-22/view-custom-metrics-from-cli

We are also currently blocking anything that is not in the hard coded list, we dont want to block custom metrics (ie: training_total). If user types a command that doesn't exist, return empty data instead.

We are currently showing a hard coded list of options for the get command, we want to add known custom metrics to this list for the help text.

@robghchen robghchen marked this pull request as draft November 20, 2020 22:56
default=(constants.BuiltinMetrics.cpu_percentage, constants.BuiltinMetrics.memory_usage),
help="One or more metrics that you want to read. Defaults to cpuPercentage and memoryUsage",
help=("One or more metrics that you want to read. Defaults to cpuPercentage and memoryUsage. Custom metrics are {}".format("hello")), # TODO replace "hello" with custom metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's instruct the user to use gradient metrics experiment metrics list to see available custom metrics, and instead we should show the user the contents of constants.METRICS_MAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh nice then i'm 5 min away from completing this

@robghchen robghchen marked this pull request as ready for review November 23, 2020 20:36
Copy link

@tallyw00d tallyw00d left a comment

Choose a reason for hiding this comment

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

command validations relaxed. LGTM :shipit:

@robghchen robghchen merged commit 5949823 into master Nov 23, 2020
@robghchen robghchen deleted the CR2-50-show-custom-metrics-options-in-get-command branch November 23, 2020 20:56
@PSBOT
Copy link

PSBOT commented Nov 23, 2020

🎉 This PR is included in version 1.2.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@PSBOT PSBOT added the released label Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants