revert the addition of ems_ref to container image #38
Conversation
|
ping @Fryguy @simon3z @moolitayer |
| @@ -1,6 +1,5 @@ | |||
| class AddStiToContainerImage < ActiveRecord::Migration[5.0] | |||
| def change | |||
| add_column :container_images, :type, :string | |||
| add_column :container_images, :ems_ref, :string | |||
There was a problem hiding this comment.
Please add a new migration @enoodle.
That way developers that already ran the migration will remove the new column
enoodle
force-pushed
the
revert_ems_ref_for_container_images
branch
from
88dbeb4 to
87949eb
Compare
|
@moolitayer I created a new migration like you asked. PTAL |
|
Checked commit enoodle@87949eb with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@enoodle @moolitayer now that's in, would it hurt to just keep it instead (not using it). Just in case we'll find later that we need it? |
|
@simon3z If we leave it always empty it will be ok. |
I'm not so sure about that. ems_ref is expected to be unique. If you don't fill it in with something, then all images will be the same reference which will likely confuse the save_inventory / graph_persistor. |
We are using it in save currently (and of course we should not) |
Oh my, terrible scribal error, we are NOT using it, sorry @enoodle the save_inventory_container code: https://github.com/ManageIQ/manageiq/blob/8ad46ceb2e51ba4fc9a0ba9f8cf6c1c228da675e/app/models/ems_refresh/save_inventory_container.rb#L222 |
|
LGTM 👍 cc @Fryguy |
Following the discussion in #21 we should revert the addition of
ems_refcolumn to container images.