Allow rake update:ui to work without db connection

[NickLaMuro]

The inherited task of webpack:compile, has a prerequisite of the 'environment' task, which will load the entire Rails environment, and attempt to load the database configuration to do so.

The task itself doesn't actually require a connection to the database, but uses the built in rails task of 'environment' to load up the application gain access to some configuration settings it depends on. When building an appliance or a docker image for manageiq, the there is no database configuration is available for this task, so it will fail in an attempt to load the database config when running the environment task.

Using an existing mechanism already used in other places in manageiq:

EvmRakeHelper.with_dummy_database_url_configuration do
  # code here that requires a DB connection
end

We can stub out the database config and allow the task to run, without the need for a config/database.yml file to be present. The rest of the rake task should function as it has.

Links

• reported error with the build
• EvmRakeHelper.with_dummy_database_url_configuration
• EvmRakeHelper.with_dummy_database_url_configuration in practice

Steps for Testing/QA

• Move or delete your config/database.yml file in your manageiq dir (or spec/manageiq dir, depending on where you are running this from)
• Run bundle exec rake update:ui
• The tasks should run without error

On master, the steps above should fail with the following error:

$ bundle exec rake update:ui
Cannot load `Rails.application.database_configuration`:
Could not load database configuration. No such file - ["config/database.yml"]

I was pretty defensive about this change and hope I didn't break something somewhere else, so hopefully this will work in other scenarios where webpack:compile might be called. That said, the above isn't an exhaustive set of tests, so you might want to do others.

[miq-bot]

Checked commit NickLaMuro@31a2420 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

[NickLaMuro]

@miq-bot assign @himdel
@miq-bot add_label blocker
@miq-bot add_label bug

See https://gitter.im/ManageIQ/manageiq/ui?at=598cde99a7b406262d78c8ca for the build error. @simaishi is currently blocked by this (thought she might have a workaround).

Sidenote: I there wasn't a build label in this repo, or I would have added that as well.

cc @martinpovolny

[jrafanie]

@NickLaMuro Very interesting... but... why is environment making a connection to the db? That's the real problem. We have an entire rake task that's supposed to fail as part of each travis build if loading rails environment using a invalid database configuration tries to connect with that config.

[NickLaMuro]

@NickLaMuro Very interesting... but... why is environment making a connection to the db? That's the real problem.

@jrafanie To start, see the link in the PR description (also linked in #1887 (comment)), and the comment in the code where I said this could take place:

# When available, run the `webpack:compile` tasks without a fully loaded
# environment, since when doing an appliance/docker build, a database isn't
# available for the :environment task (prerequisite for
# 'webpacker:compile') to function.

To that end, rake update:ui is part of the Dockerfile (and I can only assume, in a similar fashion, the appliance build), and gets run before a database.yml is present.

We have an entire rake task that's supposed to fail as part of each travis build if loading rails environment using a invalid database configuration tries to connect with that config.

While true, as you have highlighted, it only tests that when that task in particular is run in isolation. It will invoke Rake::Task["environment"] with EvmRakeHelper.with_dummy_database_url_configuration wrapping it, but our Rakefile doesn't modify rake environment so that it will always do this. It only tests that using EvmRakeHelper.with_dummy_database_url_configuration with rake environment will work without a database, not that this is always the case (because that would actually be a bad thing otherwise).

Again, also mentioned in the PR description, this is the same thing that we do in rake evm:assets:compile, so I am not doing anything terribly different here either.

[NickLaMuro]

Woops!

I did forget to reference that the environment task is not coming from something we own, but from rails/webpacker, which is why I am wrapping the call of webpack:compile (which we own), which is a wrapper around webpacker:compile (which is owned by rails/webpacker).

Sorry if that wasn't clear.

[martinpovolny]

Looks great to me, but leaving it for @himdel.

[himdel]

@NickLaMuro Any reason you're wrapping the call of webpack:compile, instead of the body?

This way, this only works when you call it via update:ui, what about something like..

--- a/lib/tasks/manageiq/ui_tasks.rake
+++ b/lib/tasks/manageiq/ui_tasks.rake
@@ -26,9 +26,11 @@ namespace :webpack do
 
   [:compile, :clobber].each do |webpacker_task|
     task webpacker_task do
-      Dir.chdir ManageIQ::UI::Classic::Engine.root do
-        Rake::Task["webpack:paths"].invoke
-        Rake::Task["webpacker:#{webpacker_task}"].invoke
+      EvmRakeHelper.with_dummy_database_url_configuration do
+        Dir.chdir ManageIQ::UI::Classic::Engine.root do
+          Rake::Task["webpack:paths"].invoke
+          Rake::Task["webpacker:#{webpacker_task}"].invoke
+        end
       end
     end
   end

instead? (well, + the conditional I guess, although I don't see it elsewhere..)

[himdel]

.. But given the timezone overlap and the broken build, I'm merging as is, we can always move this later.

And it does work for me :).

[simaishi]

@NickLaMuro Thank you for quickly fixing this!

[NickLaMuro]

instead? (well, + the conditional I guess, although I don't see it elsewhere..)

@himdel DOH! That would have been much better/more portable... yeah, 100% agree. The only excuses I have is:

a) It was late in the day when I finally got this working
b) My original thought was to try and enhance the webpacker:compile task, and then I realized this would be simpler and much less work.

So yeah, didn't think of what you had there, but that really should have been what we did. I can (or you can) open of a change to make that the preferred method instead, because I do agree, this really should have been done as you suggested. Good call!

[jrafanie]

@NickLaMuro Sorry, I was confused by this in the description:

The inherited task of webpack:compile, has a prerequisite of the 'environment' task, which will load the entire Rails environment, and make a DB connection to do so.

It doesn't actually try to connect to the database, it fails when it tries to retrieve the database configuration via:

Cannot load `Rails.application.database_configuration`:
Could not load database configuration. No such file - ["config/database.yml"]

I'm good with the change, but was confused since it read it as something is actually trying to connect and possibly run a query just loading environment, which would be a totally different problem.

[NickLaMuro]

@jrafanie ah, good point, I will make that edit in the description, as I can see how that is confusing.