[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