[ARVADOS] created: 8295ba721133c341fd72350f7ca9438d1a99cdbe
git at public.curoverse.com
git at public.curoverse.com
Thu Jan 8 10:25:37 EST 2015
at 8295ba721133c341fd72350f7ca9438d1a99cdbe (commit)
commit 8295ba721133c341fd72350f7ca9438d1a99cdbe
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date: Thu Jan 8 10:26:35 2015 -0500
4924: Refactor arv edit and arv create to improve error handling.
* Error messages are now added in a comment block at the top of the file, and
the file is re-opened in the user's editor.
* Does not try to update attributes that are not changed.
* Exiting the editor with the file unchanged exits the editing loop.
diff --git a/sdk/cli/bin/arv b/sdk/cli/bin/arv
index 533ea39..96acd01 100755
--- a/sdk/cli/bin/arv
+++ b/sdk/cli/bin/arv
@@ -153,70 +153,109 @@ def check_subcommands client, arvados, subcommand, global_opts, remaining_opts
end
end
-def arv_edit_save_tmp tmp
- FileUtils::cp tmp.path, tmp.path + ".saved"
- puts "Saved contents to " + tmp.path + ".saved"
-end
-
def command_exists?(command)
- ENV['PATH'].split(':').each {|folder| File.executable?(File.join(folder, command))}
+ File.executable?(command) || ENV['PATH'].split(':').select {|folder| File.executable?(File.join(folder, command))}.any?
end
-def run_editor tmp_file, global_opts
- need_edit = true
- while need_edit
- pid = Process::fork
- if pid.nil?
- editor = nil
- [ENV["VISUAL"], ENV["EDITOR"], "nano", "vi"].each do |e|
- editor ||= e if e and command_exists? e
- end
- if editor.nil?
- puts "Could not find any editor to use, please set $VISUAL or $EDITOR to your desired editor."
- exit 1
- end
- exec editor, tmp_file.path
- else
- Process.wait pid
+def run_editor path
+ pid = Process::fork
+ if pid.nil?
+ editor = nil
+ [ENV["VISUAL"], ENV["EDITOR"], "nano", "vi"].each do |e|
+ editor ||= e if e and command_exists? e
+ end
+ if editor.nil?
+ abort "Could not find any editor to use, please set $VISUAL or $EDITOR to your desired editor."
end
+ exec editor, path
+ else
+ Process.wait pid
+ end
+
+ if $?.exitstatus != 0
+ raise "Editor exited with status #{$?.exitstatus}"
+ end
+end
- if $?.exitstatus == 0
- tmp_file.open
- newcontent = tmp_file.read()
+def edit_and_commit_object initial_obj, tmp_stem, global_opts, &block
- newobj = {}
+ content = case global_opts[:format]
+ when 'json'
+ Oj.dump(initial_obj, :indent => 1)
+ when 'yaml'
+ initial_obj.to_yaml
+ else
+ abort "Unrecognized format #{global_opts[:format]}"
+ end
+
+ tmp_file = Tempfile.new([tmp_stem, ".#{global_opts[:format]}"])
+ tmp_file.write(content)
+ tmp_file.close
+
+ begin
+ error_text = ''
+ while true
begin
- case global_opts[:format]
- when 'json'
- newobj = Oj.load(newcontent)
- when 'yaml'
- newobj = YAML.load(newcontent)
- end
- need_edit = false
- rescue Exception => e
- n = 1
- newcontent.each_line do |line|
- puts "#{n.to_s.rjust 4} #{line}"
- n += 1
- end
- puts "Parse error! " + e.to_s
- puts "\nTry again (y/n)? "
- yn = "X"
- while not ["y", "Y", "n", "N"].include?(yn)
- yn = $stdin.read 1
- end
- if yn == 'n' or yn == 'N'
- arv_edit_save_tmp tmp_file
- abort
+ run_editor tmp_file.path
+
+ tmp_file.open
+ newcontent = tmp_file.read()
+ tmp_file.close
+
+ # Strip lines starting with '#'
+ newcontent = newcontent.lines.select {|l| !l.start_with? '#'}.join
+
+ # Load the new object
+ newobj = case global_opts[:format]
+ when 'json'
+ Oj.load(newcontent)
+ when 'yaml'
+ YAML.load(newcontent)
+ end
+
+ yield newobj
+
+ break
+ rescue => e
+ puts "Error: #{e}"
+ tmp_file.open
+ newcontent = tmp_file.read()
+ tmp_file.close
+
+ if newcontent == error_text
+ FileUtils::cp tmp_file.path, tmp_file.path + ".saved"
+ puts "File is unchanged, edit aborted."
+ abort "Saved contents to " + tmp_file.path + ".saved"
+ else
+ tmp_file.open
+ tmp_file.truncate 0
+ error_text = e.to_s.lines.map {|l| '# ' + l}.join + "\n"
+ error_text += "# Please fix the error and try again.\n"
+ error_text += newcontent.lines.select {|l| !l.start_with? '#'}.join
+ tmp_file.write error_text
+ tmp_file.close
end
end
- else
- puts "Editor exited with status #{$?.exitstatus}"
- exit $?.exitstatus
end
+ ensure
+ tmp_file.close(true)
end
- newobj
+ nil
+end
+
+def check_response result
+ begin
+ results = JSON.parse result.body
+ rescue JSON::ParserError => e
+ raise "Failed to parse server response:\n" + e.to_s
+ end
+
+ if result.response.status != 200
+ raise "#{result.response.status}: " + (results['errors'] && results['errors'].join('\n') || "")
+ end
+
+ results
end
def arv_edit client, arvados, global_opts, remaining_opts
@@ -243,7 +282,7 @@ def arv_edit client, arvados, global_opts, remaining_opts
if /^[a-f0-9]{32}/.match uuid
abort "Arvados collections are not editable."
else
- abort "#{n} does not appear to be an Arvados uuid"
+ abort "#{uuid} does not appear to be an Arvados uuid"
end
end
@@ -260,79 +299,36 @@ def arv_edit client, arvados, global_opts, remaining_opts
abort "Could not determine resource type #{m[2]}"
end
- api_method = 'arvados.' + rsc + '.get'
-
- result = client.execute(:api_method => eval(api_method),
- :parameters => {"uuid" => uuid},
- :authenticated => false,
- :headers => {
- authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
- })
begin
- results = JSON.parse result.body
- rescue JSON::ParserError => e
- abort "Failed to parse server response:\n" + e.to_s
+ result = client.execute(:api_method => eval('arvados.' + rsc + '.get'),
+ :parameters => {"uuid" => uuid},
+ :authenticated => false,
+ :headers => {
+ authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
+ })
+ oldobj = check_response result
+ rescue => e
+ abort "Server error: #{e}"
end
if remaining_opts.length > 0
- results.select! { |k, v| remaining_opts.include? k }
- end
-
- content = ""
-
- case global_opts[:format]
- when 'json'
- content = Oj.dump(results, :indent => 1)
- when 'yaml'
- content = results.to_yaml
- end
-
- tmp_file = Tempfile.new([uuid, "." + global_opts[:format]])
- tmp_file.write(content)
- tmp_file.close
-
- newobj = run_editor tmp_file, global_opts
-
- begin
- if newobj != results
- api_method = 'arvados.' + rsc + '.update'
- dumped = Oj.dump(newobj)
-
- begin
- result = client.execute(:api_method => eval(api_method),
- :parameters => {"uuid" => uuid},
- :body_object => { rsc.singularize => dumped },
- :authenticated => false,
- :headers => {
- authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
- })
- rescue Exception => e
- puts "Error communicating with server, error was #{e}"
- puts "Update body was:"
- puts dumped
- arv_edit_save_tmp tmp_file
- abort
- end
-
- begin
- results = JSON.parse result.body
- rescue JSON::ParserError => e
- arv_edit_save_tmp tmp_file
- abort "Failed to parse server response:\n" + e.to_s
- end
-
- if result.response.status != 200
- puts "Update failed. Server responded #{result.response.status}: #{results['errors']} "
- puts "Update body was:"
- puts dumped
- arv_edit_save_tmp tmp_file
- abort
- end
+ oldobj.select! { |k, v| remaining_opts.include? k }
+ end
+
+ edit_and_commit_object oldobj, uuid, global_opts do |newobj|
+ newobj.select! {|k| newobj[k] != oldobj[k]}
+ if !newobj.empty?
+ result = client.execute(:api_method => eval('arvados.' + rsc + '.update'),
+ :parameters => {"uuid" => uuid},
+ :body_object => { rsc.singularize => newobj },
+ :authenticated => false,
+ :headers => {
+ authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
+ })
+ check_response result
else
puts "Object is unchanged, did not update."
end
- ensure
- tmp_file.close(true)
end
exit 0
@@ -379,60 +375,21 @@ def arv_create client, arvados, global_opts, remaining_opts
end
end
-
- newobj = {}
+ initial_obj = {}
if create_opts[:project_uuid]
- newobj["owner_uuid"] = create_opts[:project_uuid]
- end
-
- case global_opts[:format]
- when 'json'
- content = Oj.dump(newobj, :indent => 1)
- when 'yaml'
- content = newobj.to_yaml
- end
-
- tmp_file = Tempfile.new(["", ".#{global_opts[:format]}"])
- tmp_file.write(content)
- tmp_file.close
-
- newobj = run_editor tmp_file, global_opts
-
- begin
- api_method = 'arvados.' + rsc + '.create'
- dumped = Oj.dump(newobj)
-
- result = client.execute(:api_method => eval(api_method),
- :parameters => method_opts,
- :body_object => {object_type => newobj},
- :authenticated => false,
- :headers => {
- authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
- })
-
- begin
- results = JSON.parse result.body
- rescue JSON::ParserError => e
- arv_edit_save_tmp tmp_file
- abort "Failed to parse server response:\n" + e.to_s
- end
-
- if result.response.status != 200
- puts "Create failed. Server responded #{result.response.status}: #{results['errors']} "
- puts "Create body was:"
- puts dumped
- arv_edit_save_tmp tmp_file
- abort
- end
-
- begin
- puts "Created object #{results['uuid']}"
- rescue
- arv_edit_save_tmp tmp_file
- abort "Unexpected response:\n#{results}"
- end
- ensure
- tmp_file.close(true)
+ initial_obj["owner_uuid"] = create_opts[:project_uuid]
+ end
+
+ edit_and_commit_object initial_obj, "", global_opts do |newobj|
+ result = client.execute(:api_method => eval('arvados.' + rsc + '.create'),
+ :parameters => method_opts,
+ :body_object => {object_type => newobj},
+ :authenticated => false,
+ :headers => {
+ authorization: 'OAuth2 '+ENV['ARVADOS_API_TOKEN']
+ })
+ results = check_response result
+ puts "Created object #{results['uuid']}"
end
exit 0
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list