Story #8087

[SDKs] `arv create` can read JSON from a file, skipping an interactive editor

Added by Nico César almost 4 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Target version:
Start date:
02/23/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

I currently see:

nico@shell:~$ arv create pipeline_template --help
Arvados command line client
Usage: arv create [--project-uuid] pipeline_template [create parameters]

This method supports the following parameters:

  -e, --ensure-unique-name      Adjust name to ensure uniqueness instead of returning an error on (owner_uuid, name)
                              collision.
  -h, --help                  Show this message

and if I execute arv create pipeline_template will open the editor.

I'd like to be able to do arv create pipeline_template --from-file file.json and that's the template file to use. This is for automation purposes of pipeline template creation from ansible.


Subtasks

Task #8519: Review PR #43ResolvedJoshua Randall

Associated revisions

Revision 548e0c54
Added by Radhika Chippada over 3 years ago

closes #8087
Merge branch 'wtsi-hgi-8087-arv-cli-request-body-from-file'

History

#1 Updated by Brett Smith almost 4 years ago

  • Tracker changed from Bug to Story
  • Subject changed from Create a pipeline template from a file to [SDKs] `arv create` can read JSON from a file, skipping an interactive editor
  • Category set to SDKs

#2 Updated by Brett Smith almost 4 years ago

  • Target version set to Arvados Future Sprints

#3 Updated by Brett Smith almost 4 years ago

You can get the same functionality today by running arv pipeline_template create --pipeline_template "$(cat input_file.json)".

#4 Updated by Joshua Randall almost 4 years ago

Brett, it's true that the workaround works for small payloads, but for larger ones you wind up with:

arv: Argument list too long

(trying to use `arv` to create a collection with a large manifest that can't be fit onto the command line)

#5 Updated by Joshua Randall almost 4 years ago

I have implemented a simple modification to the CLI that allows supplying of the request body from a file (or stdin) rather than requiring it be specified on the command line. This is implemented by first attempting to use `JSON.parse` to parse the supplied value request body argument as JSON. If JSON parsing succeeds, the value is handled as before and sent as the request body. If parsing fails, we assume the string is actually a filename (or the special value '-' to indicate stdin) and read the contents of that file (or stdin) and use that as the request body instead.

https://github.com/curoverse/arvados/pull/43

#6 Updated by Joshua Randall almost 4 years ago

  • Status changed from New to Feedback
  • Assigned To set to Joshua Randall

#7 Updated by Joshua Randall almost 4 years ago

With the changes in the PR, all of the following would be equivalent:

arv pipeline_template create --pipeline_template "$(cat input_file.json)" 
arv pipeline_template create --pipeline_template input_file.json
cat input_file.json | arv pipeline_template create --pipeline_template -

But the latter two will work for arbitrarily large `input_file.json`, whereas the first one will be limited to the maximum command line length.

#8 Updated by Joshua Randall almost 4 years ago

  • Assigned To deleted (Joshua Randall)

#9 Updated by Joshua Randall almost 4 years ago

Updated PR 43 (https://github.com/curoverse/arvados/pull/43) based on discussion on IRC. Adds additional safety check - refuses to proceed if the provided argument is both valid JSON and also a readable file.

#10 Updated by Brett Smith almost 4 years ago

  • Assigned To set to Joshua Randall

#11 Updated by Brett Smith over 3 years ago

  • Target version changed from Arvados Future Sprints to 2016-03-30 sprint

#12 Updated by Brett Smith over 3 years ago

  • Story points set to 0.5

#13 Updated by Radhika Chippada over 3 years ago

Review comments for branch wtsi-hgi-8087-arv-cli-request-body-from-file

  • The error message displayed when a wrong json is provided at command line is misleading. It should say “invalid json”. May be we can say neither valid json nor existing filename.
    arv pipeline_template create --pipeline-template '{"name":"does not end properly}'
    File '{"name":"does not end properly}' specified for option '--pipeline_template' does not exist.
    
  • When the “yes it is readable file” path is being executed, can we please check if the resource_body from the file is a valid json or not? There is no point in sending the request to the API server when the file content is not a valid json. ( else block at line # 660 )
  • This is not really an issue with functionality. But can you please rename _is_json, _resource_opt_desc etc to not start with an underscore. As far as I know, we do not use this naming convention. I do appreciate the descriptive names though :)
  • Can we not print the potentially long string twice here (suggest removing the first #{resource_body} to make reading the message easier.
    • Current: Argument '#{resource_body}' specified for option '--#{resource_schema.to_sym}' is both valid JSON and a readable file, cannot continue (suggest renaming the file '#{resource_body}
    • Suggested: Argument specified for option '--#{resource_schema.to_sym}' is both valid JSON and a readable file. Aborting. Please consider renaming the file '#{resource_body}
  • Trailing white space for this comment line “we don't actually need the results of the parsing”
  • Just an observation. The example command listed in note 7 should use --pipeline-template ( not _ )
  • Would it be too much if I make the observation that we do not have an automated test for this? Please consider adding a test in test_arv-collection-create.rb. If your time does not permit, I can try to add it little later in the sprint.

Thanks.

#14 Updated by Brett Smith over 3 years ago

  • Target version changed from 2016-03-30 sprint to Arvados Future Sprints

#15 Updated by Radhika Chippada over 3 years ago

  • Target version changed from Arvados Future Sprints to 2016-06-08 sprint

#16 Updated by Radhika Chippada over 3 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:548e0c54db524cb7317850d4dfd8f3ee0b93cdb0.

Also available in: Atom PDF