Allow proper targeted refresh #272
Conversation
Add correct targeted queries for the complex ICs
We can use targeted => true and targeted specs are still passing
|
@agrare @gtanzillo and we need to build proper targeted_arel queries. Although I think those can be also auto-generated later. So with this, the targeted persistor should be working (the specs probably do not have 100% coverage, so as we do streaming refresh, we'll be adding more specs) |
| ) | ||
|
|
||
| targeted_arel = lambda do |inventory_collection| | ||
| # TODO(lsmola): if we use :association ^ instead of :arel, this can be autogenerated | ||
| p_collection = inventory_collection.parent_inventory_collections.first |
There was a problem hiding this comment.
is this distinct from parent_collection defined above?
There was a problem hiding this comment.
should be the same, but it's safer to load it from the inventory_collection context
| p_collection = inventory_collection.parent_inventory_collections.first | ||
| rel = p_collection.db_collection_for_comparison_for(p_collection.targeted_scope.primary_references) | ||
|
|
||
| query = Tagging.where( |
There was a problem hiding this comment.
the query = part here does nothing, you just need return value from the lambda, right?
| end | ||
|
|
||
| def shared_options |
There was a problem hiding this comment.
it's defined in the base persister
Remove unused assignment
|
Checked commits Ladas/manageiq-providers-kubernetes@b6a7c7c~...995524e with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
I'm not following the details of what p_collection.db_collection_for_comparison_for(p_collection.targeted_scope.primary_references) would resolve to exactly, but looks plausible :-)
Why is it that the parametric add_foos() functions are exactly the places that needed a targetted_arel?
There is some duplication between the regular queries and their targeted counterparts but it's not easy to DRY, would just obfuscate the code.
|
@cben the targeted_arel is needed because we are not able to build it automatically. Instead of DRYing it, we can rather rewrite the :arel attributes to relations on ems. Then the targeted_arel should be able to generate itself. :-) |
|
@JPrause the Milestone github offers me here is Sprint 92 ending yesterday — should I use that one? Is it normal that 93 doesn't exist yet? |
|
@agrare @gtanzillo I merged assuming you already reviewed this and I'm the one this is blocked on, but seems not (or you didn't say anything)? |
|
@cben yes, please use 92. The milestones are closed Tuesday mornings. If 92 gets closed before you can assign, you can use 93. |
|
@cben I should have said,...every two weeks on the Tuesday morning before sprint review. |
Depends on:
Allow proper targeted refresh. For passing specs, we must define the correct targeted_arel queries.