Fix idempotency for publications #85
Conversation
There was a problem hiding this comment.
This is a problem, but i don't think it's safe to assume, that the first one found is the one you want.
The fact that there is no way to identify publications with the same key as you create them is the real problem here for idempotence.
But as your reproducer already shows: If you stay within the world of idempotence (ansible) you cannot produce that situation. So I'd opt for letting the module fail instead of creating a third publication.
plugins/module_utils/pulp.py
Outdated
| if search_result["count"] == 1: | ||
| # While we limit to one result, the count may be more than one. | ||
| # This may happen if the parameters are not unique, e.g. for a publication. | ||
| if search_result["count"] >= 1: |
There was a problem hiding this comment.
I think we should fail if we find more than one item.
There was a problem hiding this comment.
I wondered if you might say this.
There was a problem hiding this comment.
At least we agree that creating the third publication is the worst behaviour.
There was a problem hiding this comment.
I was 50/50 on which way to go with the solution tbh
markgoddard
force-pushed
the
issues/84
branch
from
1507973
to
0d8e970
Compare
plugins/module_utils/pulp.py
Outdated
| # While we limit to one result, the count may be more than one. | ||
| # This may happen if the parameters are not unique, e.g. for a publication. |
There was a problem hiding this comment.
Can you move that comment to the elif clause, and state that this can happen for models with insufficient or not existing "natural key" (that is how it is called in Django) e.g. Publications.
If there are multiple publications of a repository version, then the rpm_publication module will create another, rather than keeping or updating one of the existing ones. This happens due to the find() method on PulpEntity looking for count==1. It uses a limit of one, so only one result is returned, but the count still gives the full number of matching items, which in this case is 2. This change fixes the issue by failing when we cannot find a unique entity matching the requested parameters. Fixes: pulp#84
markgoddard
force-pushed
the
issues/84
branch
from
0d8e970
to
822f057
Compare
| elif search_result["count"] > 1: | ||
| # While we limit to one result, the count may be more than one. | ||
| # This may happen for models with an insufficient or non-existent "natural key" | ||
| # e.g. for a publication. |
If there are multiple publications of a repository version, then the
rpm_publication module will create another, rather than keeping or
updating one of the existing ones.
This happens due to the find() method on PulpEntity looking for
count==1. It uses a limit of one, so only one result is returned, but
the count still gives the full number of matching items, which in this
case is 2.
This change fixes the issue by failing when we cannot find a unique
entity matching the requested parameters.
Fixes: #84