[ARVADOS] updated: 1974e0e333395ee3eccedc2410441a7b018187da

git at public.curoverse.com git at public.curoverse.com
Wed Jan 14 01:17:43 EST 2015


Summary of changes:
 docker/base/Dockerfile |   2 +-
 sdk/cli/bin/arv        | 318 +++++++++++++++++++++++--------------------------
 2 files changed, 148 insertions(+), 172 deletions(-)

  discards  6b8e471dcc83ecb867dc4ba7358e9bfd90341049 (commit)
  discards  e0d047ba6f48708550d7a53644d2affacbfc2e43 (commit)
  discards  15ae2d2caa3209003648296385566381cfc0076e (commit)
  discards  5ebd32d67d0bcb26ae034e01bc928795bb0fcfe7 (commit)
  discards  cedde3e5d83a44a4e312d17d313a8eae96885962 (commit)
       via  1974e0e333395ee3eccedc2410441a7b018187da (commit)
       via  f4cd3a9893e2f4a0b6fffcd6824a422e3d603b96 (commit)
       via  eec57e32699568c315ca1134e9186a04c76d2359 (commit)
       via  f2834f4447a9b9b601c0b4f49434a77f9f054e50 (commit)
       via  2d095685cb582285e7ed89c7a984e5277aae0104 (commit)
       via  5f4ca24ce0d52857901664c3bb5e228316af4d74 (commit)
       via  138a4b0a5de177faede5255841a5c6fca06b31f4 (commit)
       via  cb15699af6eb0cad3560280b741a135b9af57a80 (commit)
       via  9fea1c7774bd256788ee76385c0eab05d4508796 (commit)
       via  5fa38c36644bd122b8c31601aa864f05bf4fba73 (commit)
       via  198963abf16d86d809a6a9fde36bc9ffd74685fd (commit)
       via  8295ba721133c341fd72350f7ca9438d1a99cdbe (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (6b8e471dcc83ecb867dc4ba7358e9bfd90341049)
            \
             N -- N -- N (1974e0e333395ee3eccedc2410441a7b018187da)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 1974e0e333395ee3eccedc2410441a7b018187da
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 10 02:55:55 2015 -0500

    3021: Use Oj to encode API responses, and to decode them in tests.
    
    * We use the Oj and multi_json gems, which makes Oj the default JSON
      parser. However, Rails' ActiveRecord::Base overrides this and uses
      the native JSON parser, which is slow. In our case we have two
      render() calls that represent nearly all cases where we ask
      ActiveRecord to serialize for us. In both cases we already have a
      hash (not a model object), and we always want JSON responses. So we
      can fix the performance problem simply by calling Oj.dump()
      ourselves, and passing the resulting JSON (instead of the hash) to
      render().
    
    More gory details:
    
    * "ActiveRecord::Base.extend kills JSON performance":
      https://github.com/rails/rails/issues/9212
    
    * "when freedom patches fight, nobody wins":
      https://github.com/intridea/multi_json/pull/138#issuecomment-24468223

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 54d5adb..a771042 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -77,7 +77,9 @@ class ApplicationController < ActionController::Base
   end
 
   def show
-    render json: @object.as_api_response(nil, select: @select)
+    render(text: Oj.dump(@object.as_api_response(nil, select: @select),
+                         mode: :compat).html_safe,
+           content_type: 'application/json')
   end
 
   def create
@@ -441,7 +443,8 @@ class ApplicationController < ActionController::Base
         except(:limit).except(:offset).
         count(:id, distinct: true)
     end
-    render json: @object_list
+    render(text: Oj.dump(@object_list, mode: :compat).html_safe,
+           content_type: 'application/json')
   end
 
   def remote_ip
diff --git a/services/api/test/test_helper.rb b/services/api/test/test_helper.rb
index 216dd2d..5ea6e62 100644
--- a/services/api/test/test_helper.rb
+++ b/services/api/test/test_helper.rb
@@ -25,7 +25,7 @@ require 'rails/test_help'
 
 module ArvadosTestSupport
   def json_response
-    ActiveSupport::JSON.decode @response.body
+    Oj.load response.body
   end
 
   def api_token(api_client_auth_name)

commit f4cd3a9893e2f4a0b6fffcd6824a422e3d603b96
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 10 02:54:22 2015 -0500

    3021: Do not compute etag for initial model state unless/until actually needed.
    
    Profiling shows that making a copy of the attributes hash (which
    modelinstance.attributes() does) is much faster than
    md5(attrs.inspect). Given that md5(attrs_when_loaded) is never needed
    in the common case where a model gets loaded but never changed or
    written back to the database, it's better to just stash a copy of the
    attributes hash when loading, and defer computing the md5 until
    it's time to write a log entry.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index a3e8ec6..38a46de 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -523,8 +523,8 @@ class ArvadosModel < ActiveRecord::Base
   end
 
   def log_start_state
-    @old_etag = etag
-    @old_attributes = logged_attributes
+    @old_attributes = attributes
+    @old_logged_attributes = logged_attributes
   end
 
   def log_change(event_type)
@@ -543,14 +543,14 @@ class ArvadosModel < ActiveRecord::Base
 
   def log_update
     log_change('update') do |log|
-      log.fill_properties('old', @old_etag, @old_attributes)
+      log.fill_properties('old', etag(@old_attributes), @old_logged_attributes)
       log.update_to self
     end
   end
 
   def log_destroy
     log_change('destroy') do |log|
-      log.fill_properties('old', @old_etag, @old_attributes)
+      log.fill_properties('old', etag(@old_attributes), @old_logged_attributes)
       log.update_to nil
     end
   end
diff --git a/services/api/lib/kind_and_etag.rb b/services/api/lib/kind_and_etag.rb
index 89c01ef..04fdca4 100644
--- a/services/api/lib/kind_and_etag.rb
+++ b/services/api/lib/kind_and_etag.rb
@@ -14,7 +14,7 @@ module KindAndEtag
     self.class.kind
   end
 
-  def etag
-    Digest::MD5.hexdigest(self.inspect).to_i(16).to_s(36)
+  def etag attrs=nil
+    Digest::MD5.hexdigest((attrs || self.attributes).inspect).to_i(16).to_s(36)
   end
 end

commit eec57e32699568c315ca1134e9186a04c76d2359
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 10 02:18:00 2015 -0500

    3021: Call Rails.application.eager_load! only once, not every single
    time we use the uuid prefix cache, for two reasons:
    
    * The reason it's here at all is to ensure that all descendant classes
      of ActiveRecord::Base have been defined, so we don't miss any model
      classes when building the uuid prefix cache. Therefore, it's more
      robust (and clearer) to call it from the function where the cache
      gets built, rather than calling it from a function that _uses_ the
      cache.
    
    * eager_load! is (evidently) not very efficient about noticing that
      everything is already loaded: when called too often, it showed up
      prominently in profiling results.

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index a170fb9..a3e8ec6 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -432,6 +432,7 @@ class ArvadosModel < ActiveRecord::Base
   def self.uuid_prefixes
     unless @@prefixes_hash
       @@prefixes_hash = {}
+      Rails.application.eager_load!
       ActiveRecord::Base.descendants.reject(&:abstract_class?).each do |k|
         if k.respond_to?(:uuid_prefix)
           @@prefixes_hash[k.uuid_prefix] = k
@@ -498,7 +499,6 @@ class ArvadosModel < ActiveRecord::Base
     end
     resource_class = nil
 
-    Rails.application.eager_load!
     uuid.match HasUuid::UUID_REGEX do |re|
       return uuid_prefixes[re[1]] if uuid_prefixes[re[1]]
     end

commit f2834f4447a9b9b601c0b4f49434a77f9f054e50
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 10 03:49:25 2015 -0500

    3021: Add a performance test.
    
    ActiveSupport doesn't seem to think its profiling code is compatible
    with Ruby 2.1, but it seems to work if you patch up a version check in
    activesupport:
    
    .../activesupport-3.2.17/lib/active_support/testing/performance/ruby.rb
    
    -if RUBY_VERSION.between?('1.9.2', '2.0.0')
    +if RUBY_VERSION.between?('1.9.2', '3.0.0')

diff --git a/services/api/Gemfile b/services/api/Gemfile
index a7da122..4cb5b19 100644
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -8,6 +8,7 @@ gem 'rails', '~> 3.2.0'
 group :test, :development do
   gem 'factory_girl_rails'
   gem 'database_cleaner'
+  gem 'ruby-prof'
   # Note: "require: false" here tells bunder not to automatically
   # 'require' the packages during application startup. Installation is
   # still mandatory.
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index c2b2351..5f57d10 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -174,6 +174,7 @@ GEM
     rdoc (3.12.2)
       json (~> 1.4)
     ref (1.0.5)
+    ruby-prof (0.15.2)
     rvm-capistrano (1.5.1)
       capistrano (~> 2.15.4)
     sass (3.3.4)
@@ -240,6 +241,7 @@ DEPENDENCIES
   pg_power
   puma
   rails (~> 3.2.0)
+  ruby-prof
   rvm-capistrano
   sass-rails (>= 3.2.0)
   simplecov (~> 0.7.1)
diff --git a/services/api/test/performance/browsing_test.rb b/services/api/test/performance/browsing_test.rb
deleted file mode 100644
index 3fea27b..0000000
--- a/services/api/test/performance/browsing_test.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-require 'test_helper'
-require 'rails/performance_test_help'
-
-class BrowsingTest < ActionDispatch::PerformanceTest
-  # Refer to the documentation for all available options
-  # self.profile_options = { :runs => 5, :metrics => [:wall_time, :memory]
-  #                          :output => 'tmp/performance', :formats => [:flat] }
-
-  def test_homepage
-    get '/'
-  end
-end
diff --git a/services/api/test/performance/links_index_test.rb b/services/api/test/performance/links_index_test.rb
new file mode 100644
index 0000000..a183a90
--- /dev/null
+++ b/services/api/test/performance/links_index_test.rb
@@ -0,0 +1,14 @@
+require 'test_helper'
+require 'rails/performance_test_help'
+
+class IndexTest < ActionDispatch::PerformanceTest
+  def test_links_index
+    get '/arvados/v1/links', {format: :json}, auth(:admin)
+  end
+  def test_links_index_with_filters
+    get '/arvados/v1/links', {format: :json, filters: [%w[head_uuid is_a arvados#collection]].to_json}, auth(:admin)
+  end
+  def test_collections_index
+    get '/arvados/v1/collections', {format: :json}, auth(:admin)
+  end
+end

commit 2d095685cb582285e7ed89c7a984e5277aae0104
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat Jan 10 03:35:28 2015 -0500

    Fix crash on missing return_to param.

diff --git a/services/api/lib/josh_id.rb b/services/api/lib/josh_id.rb
index a63b251..a7e8ff2 100644
--- a/services/api/lib/josh_id.rb
+++ b/services/api/lib/josh_id.rb
@@ -42,7 +42,7 @@ module OmniAuth
       end
 
       def callback_url
-        full_host + script_name + callback_path + "?return_to=" + CGI.escape(request.params['return_to'])
+        full_host + script_name + callback_path + "?return_to=" + CGI.escape(request.params['return_to'] || '')
       end
 
       def raw_info

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list