Clean Azure image names #198
Conversation
| @@ -0,0 +1,12 @@ | |||
| class AzureNormalizeImageName < ActiveRecord::Migration[5.0] | |||
| class Vm < ActiveRecord::Base | |||
There was a problem hiding this comment.
Should this be class VmOrTemplate < ActiveRecord::Base?
|
|
||
| def up | ||
| say_with_time("Updating Azure template name") do | ||
| Vm.where(:type => "ManageIQ::Providers::Azure::CloudManager::Template") |
There was a problem hiding this comment.
Should this be VmOrTemplate.where(...)?
There was a problem hiding this comment.
Works either way afaik, since I'm specifying the type.
There was a problem hiding this comment.
I think templates will not be returned by Vm.where(), I think it requires VmOrTemplate. Can you check on the console?
| Vm.where(:type => "ManageIQ::Providers::Azure::CloudManager::Template") | ||
| .update_all("name = regexp_replace(name, '^./', '')") | ||
| end | ||
| end |
There was a problem hiding this comment.
do you need a down for rollback? Or a change instead?
There was a problem hiding this comment.
It's not something that can be rolled back consistently I don't think, so no.
There was a problem hiding this comment.
I think it is risky not to add one and writing one isn't a large effort. One of the scenarios I could see as being a problem is if the user does not back up the DB before an upgrade and the upgrade has to be undone.
|
@bronaghs ok, changed it to VmOrTemplate. |
except that's an issue and not a PR...do you have a PR on the other side? |
|
Waiting on merge until the other side is ready |
|
@Fryguy - I had asked Dan to add a |
Use VmOrTemplate instead of Vm. Added down method, added specs.
|
Checked commit https://github.com/djberg96/manageiq-schema/commit/91d183c27cd0cf430a25e2734961020aad5ef762 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 db/migrate/20180507134810_azure_normalize_image_name.rb
|
Due to a quirk of File.dirname, the Azure provider can end up with leading "./" in the image name. This looks dumb and should be fixed.
Should be merged in conjunction with ManageIQ/manageiq-providers-azure#240