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

Make ems_ref of service_port_config unique #89

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Aug 11, 2017

Make ems_ref of service_port_config unique by adding :protocol.

Make ems_ref of service_port_config unique by adding :protocol.
@miq-bot
Copy link
Member

miq-bot commented Aug 11, 2017

Checked commit Ladas@5eac3f6 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@cben
Copy link
Contributor

cben commented Aug 13, 2017

Sounds good, this is what we wanted to do, but I want to reproduce this in real life.
(Please do ping me if I neglect this!)

@simon3z
Copy link
Contributor

simon3z commented Aug 24, 2017

@Ladas @cben why not just using the name? It's the unique identifier.

Also, I think that for correctness technically we'd need a migration (maybe we can get away without it, but at least let's consider it).

@Ladas
Copy link
Contributor Author

Ladas commented Aug 24, 2017

@cben can we leverage the name here as @simon3z suggests?

@cben
Copy link
Contributor

cben commented Sep 13, 2017

So just "#{service_id}_#{port_config.name}"? Sounds good.

@simon3z in ManageIQ/manageiq#15574 Ladislav concluded we're (sometimes) doing unnecessary deletes/inserts anyway so we can do them one last time, don't require a migration.
Port configs are not a user-visible collection, deletes/inserts should not cause problem.
By that logic, this is also backportable.

I keep thinking "why not first change refresh to ignore ems_ref and use individual parts", then drop ems_ref. Problem is that service_id string passed into parse_service_port_config() is not stored anywhere except in the ems_ref.
Wait, we don't need it, the port config is linked to specific parent service anyway!
Sounds like we can identify in old refersh just by [:name] and in graph refresh by :manager_ref => [:container_service, :name]?

@simon3z
Copy link
Contributor

simon3z commented Nov 6, 2017

@simon3z in ManageIQ/manageiq#15574 Ladislav concluded we're (sometimes) doing unnecessary deletes/inserts anyway so we can do them one last time, don't require a migration.

👍

Sounds like we can identify in old refersh just by [:name]

Correct me if I'm wrong but it seems what I was suggesting in my previous comment, so 👍

@Ladas
Copy link
Contributor Author

Ladas commented Nov 13, 2017

replaced by:
#168
ManageIQ/manageiq#16454

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

Successfully merging this pull request may close these issues.

4 participants