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

Have parent inventory collections as dependencies #15903

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Aug 29, 2017

Have parent inventory collections as dependencies, so we always ensure that disconnect is done via parent(dependency), even if the current collection is empty.

Add parent_inventory_collections as a dependency, to ensure
we always run disconnect by parent first, even if the
current collection is blank
Add __parent_inventory_collections as intenrnal attribute, so
it's ignored by black/white listing
@Ladas
Copy link
Contributor Author

Ladas commented Aug 29, 2017

@miq-bot assign @agrare
@miq-bot assign enhancement

@miq-bot
Copy link
Member

miq-bot commented Aug 29, 2017

@Ladas 'enhancement' is an invalid assignee, ignoring...

Add parent_inventory_collections to attributes spec
Make parent_inventory_collection mandatory only for targeted, in
the full refresh, we will tolerate a missing IC, since they each
persistor can have only subset of them.
@miq-bot
Copy link
Member

miq-bot commented Aug 29, 2017

Checked commits Ladas/manageiq@05391f9~...539beb5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@cben
Copy link
Contributor

cben commented Aug 30, 2017

Thanks for the phenomenally quick fix!
Code changes LGTM. But could you give a more detailed explanation how it interacts with deletion/disconnection — in PR description, or better in documentation for parent_inventory_collections or a test specifically for this scenario?

@Ladas
Copy link
Contributor Author

Ladas commented Aug 31, 2017

@cben I think the specs should be enough in Containers and AWS? The thing here is that if we don't pass any CustomAttribute for saving, the dependency graph can't be built, so in this case the saving of CustomAttribute was done before the saving of ContainerImage. The :parent_inventory_collections serve for having fixed dependency there, so even if we have no data, the processing of ContainerImages will be done before CustomAttributes.

So this is only for doing the right order of disconnects. And it aligns with how we set parent_inventory_collections for a targeted refresh. If that would not align in the future, we would add another field for modeling fixed dependencies for the right order of disconnects.

@cben
Copy link
Contributor

cben commented Aug 31, 2017

Ah OK, I'm not familiar yet with how targetted refresh is normally done :-)

@agrare agrare merged commit b0955e8 into ManageIQ:master Sep 5, 2017
@chessbyte chessbyte added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 5, 2017
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