-
Notifications
You must be signed in to change notification settings - Fork 125
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
Migrate Nuage CloudSubnet default type to new subclass #215
Conversation
6863e80
to
57fd653
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
@gtanzillo @agrare so this is the tiny schema PR that we were talking about in Berlin 2/2 |
57fd653
to
37e1b53
Compare
@Ladas can you please take a look again here, I've applied two changes:
|
@Fryguy can you review? ManageIQ/manageiq-providers-nuage#105 was merged |
|
||
say_with_time('Reverting Nuage L2 subnet subclass to default subnet type') do | ||
CloudSubnet.where(:type => 'ManageIQ::Providers::Nuage::NetworkManager::CloudSubnet::L2') | ||
.update_all(:type => 'ManageIQ::Providers::Nuage::NetworkManager::CloudSubnet') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miha-plesko looks like rubocop wants this also aligned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Miscusi, fixed. I knew all those "Avoid using update_all" are legit but didn't notice the Align one for some reason.
Nuage inventory was recently updated to fetch two types of CloudSubnets: ``` - ManageIQ::Providers::Nuage::NetworkManager::CloudSubnet::L3 - ManageIQ::Providers::Nuage::NetworkManager::CloudSubnet::L2 ``` Before only CloudSubnet::L3 were inventoried using default type CloudSubnet. With this migration we make sure that this default type is converted to CloudSubnet::L3 type because persister won't modify STI type per se. Signed-off-by: Miha Pleško <[email protected]>
37e1b53
to
a283008
Compare
Checked commit miha-plesko@a283008 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 db/migrate/20180607084710_nuage_subclass_l3_cloud_subnet.rb
|
At some point, we're gonna need to merge this @Fryguy or else user will have to remove and re-add Nuage provider to get the right types in DB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Nuage inventory was recently updated to fetch two types of CloudSubnets:
Before only CloudSubnet::L3 were inventoried using default type CloudSubnet. With this migration we make sure that this default type is converted to CloudSubnet::L3 type because persister won't modify STI type per se.
Related PR where we updated parser: ManageIQ/manageiq-providers-nuage#94
@Ladas thanks for pointing out that STI type won't be updated by refresher!
@miq-bot add_label data
@miq-bot assign @Ladas