Add option to skip call to purge_before_load in parallel::load_schema and parallel::load_structure

[Tolsto]

It should be possible to skip the call to purge_before_load in the parallel::load_schema and parallel::load_structure tasks as this resembles the behaviour of db:structure:load and db:schema:load tasks.

[grosser]

• why do you want to disable that ?
• any idea why this was added ... I'd guess convenience helper to make test runs not fail ... but maybe it should be disabled by default ?
• I'd prefer an ENV var to switch this since having purge as the second argument does not feel very intuitive to me ...

[Tolsto]

• We run a Rails setup in which we use both schema.rb and structure.sql. It requires us to load the structure directly after the schema. We therefore cannot purge the DB before loading the structure file.
• I agree that an ENV var is the cleaner approach, I've updated the pull request.
• I don't know why the functionality was added in the first place. Disabling it by default would make sense because it is also the expected behaviour of db:schema:load and db:structure:load. However, I feel that such a change should be announced at least one version on advance.

[grosser]

announcing things does not help anyone ... nobody is reading 'updates' :D

[grosser]

errored: NoMethodError: undefined methodlowercase' for "nil":String`

ideally also add a test that fails without this ...

[grosser]

cannot use Rails.version since this might be used without rails ... ActiveRecord::Version::STRING should work

[Tolsto]

Why would this be used without Rails? All tasks that call this method depend on Rails' rake tasks

[grosser]

I always though as this only needing ActiveRecord to work ... tasks come from https://github.com/rails/rails/blob/master/activerecord/lib/active_record/railties/databases.rake ... which is a railtie ... adding rails to the mix seems to still be unnecessary to me ... maybe ActiveRecord.version => #<Gem::Version "5.0.0.1">

[grosser]

bah ... does not work on rails 3 ... maybe ActiveRecord::VERSION::STRING is best ...

[grosser]

defining a class at runtime is a bad idea :D

[Tolsto]

How would you stub the call to Rails.version instead?

[grosser]

define it global ... and then either add it there or stub it ...

[grosser]

FYI I'd test this like so: (less nesting / context switching and 1 change per test)

it "is true" do
  exoect(subject).to eq true
end

it "is false when set to false" do
  with_env(PURGE_BEFORE_LOAD: 'false') do
    expect(subject).to eq false
  end
end

it "is true when set to true" do
  with_env(PURGE_BEFORE_LOAD: 'true') do
    expect(subject).to eq true
  end
end

[Tolsto]

I'll refactor it. However, I prefer to use expect(ENV) to set envs because it will fail if the env doesn't get used anymore because someone changed the code.

[grosser]

just FYI either way works :)