feat(rpm_repository): add retain_repo_versions parameter #171
Conversation
|
This is related to #165 |
There was a problem hiding this comment.
How do you feel about adding tests?
These modules are about to be transfered to a new framework. Adding this parameter to the tests will ensure it's not immediately broken afterwards.
(Please do not test the retain version feature, but just that the model is able to set it on a repository.)
plugins/modules/rpm_repository.py
Outdated
| @@ -41,6 +41,10 @@ | |||
| - Max number of latest versions of each package to keep (optional) | |||
| type: int | |||
| version_added: "0.0.16" | |||
| retain_repo_versions: | |||
| description: | |||
| - Max number of repository versions to keep (optional) | |||
There was a problem hiding this comment.
| - Max number of repository versions to keep (optional) | |
| - Max number of repository versions to keep |
not being required is implied by ansible. Can you adjust line 41 too?
Also version_added may be nice.
There was a problem hiding this comment.
Thanks for the feedback, @mdellweg . I've made the adjustments and will submit them soon.
Can you confirm what you would like to see regarding tests? I have produced 2 tests:
- set
retain_repo_versionsto 5 - update
retain_repo_versionsto 1 (i.e. don't keep old versions)
I have ran and recorded the tests--they all pass. However, I see the following now:
- all existing
tests/fixtures/rpm_repository-*.ymlfiles have been modified - 2 new fixture files were created:
test/fixtures/rpm_repository-{22,23}.yml
What portions of the tests/results would you like committed?
Side note:
There seems to be a limitation in which every call that includes the retain_repo_versions parameter updates the returned pulp_last_updated timestamp (no other returned values differ). This is the case even when retain_repo_versions is not changed from the previous call. I assume this is a bug in pulpcore itself; however, it does limit the ability to assert results.changed == false on repeated calls with the same value for retain_repo_versions.
There was a problem hiding this comment.
What portions of the tests/results would you like committed?
All of them. They are separate files for technical reasons only.
Side note: [...]
That actually sounds like a bug on the client side. When we don't want to change the value, we must not do a patch call, and that still seems to happen.
There was a problem hiding this comment.
Hey @mdellweg, I have changed the tests to try for idempotence and am not seeing the previous issue. I believe that I must have made an error in my original work. Please see my changes and let me know if there is anything else needed for approval.
I appreciate you allowing me to contribute to this project.
joey-grant
changed the title
joey-grant
changed the title
joey-grant
force-pushed
the
develop
branch
3 times, most recently
from
07678c0
to
74b2877
Compare
|
Looks like you need to run "make format" once. |
This change adds `retain_repo_versions` parameter option to `rpm_repository` module. It also adds tests and associated fixtures. fixes pulp#165
Hey @mdellweg, I've pushed the newly formatted code. |
This change adds
retain_repo_versionsparameter option torpm_repositorymodule.