On Fri, Sep 09, 2011 at 01:11:35AM -0400, Mo Morsi wrote:
I ran into these issues myself, and was going to post similar
feedback
before I noticed the reply. See additional comments inline
On 08/26/2011 04:55 PM, Matt Wagner wrote:
> ---
> src/db/migrate/20110329104633_rename_ssh_user.rb | 16 ++++++++++------
> .../migrate/20110513160000_rename_rhevm_target.rb | 12 ++++++++----
> ...110707082304_rename_rhevm_and_vmware_targets.rb | 6 ++++--
> .../migrate/20110811091500_rename_condorcloud.rb | 2 ++
> 4 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/src/db/migrate/20110329104633_rename_ssh_user.rb
b/src/db/migrate/20110329104633_rename_ssh_user.rb
> index e77eae3..f13ae1f 100644
> --- a/src/db/migrate/20110329104633_rename_ssh_user.rb
> +++ b/src/db/migrate/20110329104633_rename_ssh_user.rb
> @@ -1,15 +1,19 @@
> class RenameSshUser< ActiveRecord::Migration
> def self.up
> pt = ProviderType.first(:conditions => {:codename => 'ec2'})
> - pt.ssh_user = 'root'
> - pt.home_dir = '/root'
> - pt.save!
> + if pt
> + pt.ssh_user = 'root'
> + pt.home_dir = '/root'
> + pt.save!
> + end
> end
Overall these changes work, though since the seed data has been moved
to seed.rb, the migrations affected here are somewhat unnecessary (these
migrations rely on the seed data, while the seed data relies on the
migrations). Instead the changes in these migrations can be added to the
seed data themselves and these migrations can be removed all together.
Hmm, you're right that this is a little weird. The problem I've been
running into lately is that we want to support new users (via db:seed)
and existing users (via migrations). If we just did it in seeds,
existing users wouldn't get them, since our db:seed can only be run
once.
No objections to pushing these patches, but at some point we should
cleanup / consolidate what belongs in the migrations and what belongs in
the seed data.
We've kicked around the idea of dumping all our migrations and using
db:schema to populate them. Both of these are worth exploring later.
> diff --git a/src/db/migrate/20110811091500_rename_condorcloud.rb
b/src/db/migrate/20110811091500_rename_condorcloud.rb
> index 332eef5..30c4762 100644
> --- a/src/db/migrate/20110811091500_rename_condorcloud.rb
> +++ b/src/db/migrate/20110811091500_rename_condorcloud.rb
> @@ -1,5 +1,7 @@
> class RenameCondorcloud< ActiveRecord::Migration
> def self.up
> + # Not sure why this is necessary, but it is:
> + ProviderType.reset_column_information
>
Yes this stumped me for a little while as well. My current theory is
that since Rails inspects the tables at the very beginning of the script
invocation, before the column is renamed in the migration, the change
does not propogate to the model class until after we manually reset it.
Now I want to dig more into this, because this sounds like a bug to me.
Why on Earth wouldn't cached information about tables be flushed after
those tables were modified?
-- Matt