Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ResourceGroup type for Azure #131

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Update ResourceGroup type for Azure #131

merged 1 commit into from
Nov 20, 2017

Conversation

djberg96
Copy link
Contributor

This sets the :type column of the ResourceGroup table to the appropriate type. Since Azure is the only provider using this table at this time, we update all records.

This is a continuation of #123.

One of several PR's that goes towards fixing:

https://bugzilla.redhat.com/show_bug.cgi?id=1503295

end

def change
ResourceGroup.update_all(:type => 'ManageIQ::Providers::Azure::ResourceGroup')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wrap this in a say_with_time like other data migrations.

self.inheritance_column = :_type_disabled
end

def change
Copy link
Member

@Fryguy Fryguy Nov 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work in a change? I would expect this data migration only on up, and then on down, I guess it would get set back to ResourceGroup.

@djberg96
Copy link
Contributor Author

@Fryguy updated.

@blomquisg
Copy link
Member

@djberg96 can you add a down to set the type to simply "ResourceGroup"? That way if someone downs this, they get something valid in the type field.

@djberg96
Copy link
Contributor Author

@blomquisg Updated.

let(:resource_group_stub) { migration_stub :ResourceGroup }
let(:expected_type) { 'ManageIQ::Providers::Azure::ResourceGroup' }

migration_context :up do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the corresponding migration_context :down test, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Add a down method that sets the type to ResourceGroup.

Add specs for down migration.
@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2017

Checked commit https://github.com/djberg96/manageiq-schema/commit/221a0e7e5c5441571498ff393976c0e0f435af2f with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 2 offenses detected

db/migrate/20171117201519_update_resource_group_type_for_azure.rb

@Fryguy Fryguy merged commit bc49956 into ManageIQ:master Nov 20, 2017
@Fryguy Fryguy added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 20, 2017
simaishi pushed a commit that referenced this pull request Nov 20, 2017
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 1b9189c19aec44ea3dc5cdbeb70635d83a777b92
Author: Jason Frey <fryguy9@gmail.com>
Date:   Mon Nov 20 10:07:11 2017 -0500

    Merge pull request #131 from djberg96/azure_resource_group_sti
    
    Update ResourceGroup type for Azure
    (cherry picked from commit bc49956309e80d7363dc93a52d8d85cb5ed3c38d)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1515452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants