Project

General

Profile

Actions

Feature #17187

closed

[costanalyzer] feature improvements

Added by Ward Vandewege almost 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Story points:
-
Release relationship:
Auto

Description

  • specify UUIDs on command line without having to insert the word "--uuid" before each one
  • specify collection UUID, meaning look up collection.properties.container_request and look up the cost of that CR
  • output the total dollar amount on stdout

Subtasks 1 (0 open1 closed)

Task #17196: review 17187-costanalyzer-updatesResolvedWard Vandewege12/06/2020Actions
Actions #1

Updated by Ward Vandewege almost 4 years ago

  • Description updated (diff)
Actions #2

Updated by Ward Vandewege almost 4 years ago

  • Status changed from New to In Progress
Actions #3

Updated by Ward Vandewege almost 4 years ago

Ready for review at 6f5431413448f52f3ae5c88553b7f7ee1532b9fd on branch 17187-costanalyzer-updates.

Actions #4

Updated by Tom Clegg almost 4 years ago

This is way more convenient, thanks.

$ arvados-client costanalyzer -output /tmp/ca --uuid su92l-4zz18-hry71275jl44dui
...
8.95688399

Specifying multiple collections/containers would be even more convenient if I could do this:

$ arvados-client costanalyzer -output /tmp/ca su92l-4zz18-a5a0ydd2feqcobz su92l-4zz18-2fmt581375q8tv6 su92l-4zz18-w1usu1jeq0jc1wm su92l-4zz18-4g3kijra6qx19k4

(Seems like we could treat everything in flag.Args() as a UUID after parsing other flags, unless we're reserving those positional args for something else?)

After accidentally testing collection UUIDs on an older version I noticed bogus --uuid args are silently ignored:

$ arvados-client costanalyzer -output /tmp/ca --uuid bogus --uuid su92l-4zz18-hry71275jl44dui
...
8.95688399
$ echo $?
0

$ arvados-client costanalyzer -output /tmp/ca --uuid bogus
Nothing to do!
$ echo $?
0

Both of those cases should probably be considered errors.

This type assertion will panic on {"properties":{"container_request":["foo"]}} -- should do a 2-value type assertion and return an error instead.

               crUUID = value.(string)

Yet another feature request: now that I get total cost on stdout, I often don't even care about the details that get saved in the output dir, so it would be nice if -output=/dev/null or -output="" meant "don't bother writing any csv files" and I didn't need to create/specify a superfluous output dir -- perhaps that could even be the default?

Actions #5

Updated by Ward Vandewege almost 4 years ago

Ready for another look at 856fd8070951c570464dbcc2785c9be689d315b9 on the 17187-costanalyzer-updates branch.

Tom Clegg wrote:

Specifying multiple collections/containers would be even more convenient if I could do this:
(Seems like we could treat everything in flag.Args() as a UUID after parsing other flags, unless we're reserving those positional args for something else?)

I've changed it that way; uuids must now be listed at the end of the option list (and no more need to say '-uuid' first).

After accidentally testing collection UUIDs on an older version I noticed bogus --uuid args are silently ignored:
Both of those cases should probably be considered errors.

Done.

This type assertion will panic on {"properties":{"container_request":["foo"]}} -- should do a 2-value type assertion and return an error instead.

Done, nice catch!

Yet another feature request: now that I get total cost on stdout, I often don't even care about the details that get saved in the output dir, so it would be nice if -output=/dev/null or -output="" meant "don't bother writing any csv files" and I didn't need to create/specify a superfluous output dir -- perhaps that could even be the default?

Sure, done. The '-output' argument is now optional.

Actions #6

Updated by Tom Clegg almost 4 years ago

LGTM, thanks!

Actions #7

Updated by Ward Vandewege almost 4 years ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #8

Updated by Peter Amstutz over 3 years ago

  • Release set to 38
Actions

Also available in: Atom PDF