Refactor Api::Initializer

[imtayadeway]

For legacy reasons there were some quite expensive operations that were wrapped in the Api::Initializer class so their execution could be delayed until the first request. This code generates some data and uses that data to populate things like Api::Environment.normalized_attributes. By moving that code into Api::Environment and allowing it to be lazily evaluated, we achieve the same effect. What remains is merely logging which I've moved into the top level lib/api.rb, eliminating the need for this class.

To show that we are still delaying population of the normalized_attributes data until required, here's what happens when I open the console:

irb(main):001:0> Api::Environment.instance_variable_get(:@normalized_attributes)
=> nil
irb(main):002:0> Api::Environment.normalized_attributes
PostgreSQLAdapter#log_after_checkout, connection_pool: size: 16, connections: 1, in use: 1, waiting_in_queue: 0
=> {:time=>{"expires_on"=>true, "last_logon"=>true, "created_on"=>true, "updated_on"=>true, "created_at"=>true, "updated_at"=>true, "evaluated_on"=>true, "last_valid_on"=>true, "last_invalid_on"=>true, "credentials_changed_on"=>true, "commit_time"=>true, "last_import_on"=>true, "fulfilled_on"=>true, "creation_time"=>true, "last_perf_capture_on"=>true, "file_mtime"=>true, "last_scan_on"=>true, "timestamp"=>true, "last_sync_on"=>true, "last_scan_attempt_on"=>true, "retires_on"=>true, "boot_time"=>true, "state_changed_on"=>true, "ems_created_on"=>true, "retirement_last_warn"=>true, "start_time"=>true, "finish_time"=>true, "last_refresh_date"=>true, "last_metrics_update_date"=>true, "last_metrics_success_date"=>true, "scheduled_on"=>true, "last_run_on"=>true, "last_accessed_on"=>true, "started_on"=>true, "stopped_on"=>true, "last_heartbeat"=>true, "placed_at"=>true, "create_time"=>true, "install_time"=>true, "logo_updated_at"=>true, "login_logo_updated_at"=>true, "lastlogon"=>true, "lastlogoff"=>true}, :url=>{"href"=>true}, :resource=>{"image_href"=>true}, :encrypted=>{"password"=>true, "root_password"=>true, "sysprep_password"=>true, "sysprep_domain_password"=>true, "bind_pwd"=>true, "amazon_secret"=>true}}

As you can see, Rails has successfully booted without calling this expensive operation

This also allowed for a few ✂️ ✂️ ✂️

@miq-bot add-label api, refactoring
@miq-bot assign @abellotti

/cc @Fryguy

[miq-bot]

Checked commits imtayadeway/manageiq@30bd885~...edbc7c9 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. 👍

[abellotti]

👍 @imtayadeway

@Fryguy what was the tool you used to keep track of allocations upon instantiating rails/workers ?

[abellotti]

@imtayadeway see ManageIQ/manageiq-gems-pending#123 for info about webtreemap, would be nice to see the numbers for the Api Controller before and after. Thanks

[Fryguy]

Out of curiosity, is the true value used at all? If not, perhaps it would be better to use a Set over a Hash in all of these cases.

[imtayadeway]

@Fryguy I thought the same thing, but since hash lookup was faster I assumed it was there for that reason. Could we sacrifice a small performance gain for a more appropriate data structure?

[Fryguy]

A Set is just a Hash under the covers, so you don't sacrifice anything.

https://github.com/ruby/ruby/blob/trunk/lib/set.rb#L84
https://github.com/ruby/ruby/blob/trunk/lib/set.rb#L28-L46

[imtayadeway]

@Fryguy I thought this too, but when I benchmarked it was consistently slower (compared with SortedSet too). Nonetheless, I'll put a PR together so perhaps we can discuss this better there

[Fryguy]

The begin/end is redundant here. Also I'm not a fan of the mega indent, personally...I prefer

   def self.time_attributes
     @time_attributes ||=
       ApiConfig.collection... do
         
       end
   end

[Fryguy]

@imtayadeway I'm going to merge this anyway. Let's take my other comments into a separate PR.

[simaishi]

@imtayadeway I assume there is no BZ for this? Not sure if this is one of those changes that any API call will go through this and no specific test is needed (in which case, I don't think BZ is needed), or if there are something QE will need to test to cover the changes made in this PR?

[imtayadeway]

@simaishi no BZ for this. This PR was really just about code - the discussion about performance only relates to ensuring that there were no regressions in the refactoring. The existing tests should ensure that everything "still works" ;)