Support min max object size in s3_lifecycle module #2205
Conversation
…m and maximum object size settings
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 13s (non-voting) |
There was a problem hiding this comment.
Thanks for taking the time to submit this, especially for including the new integration tests.
If you take a look at the output from the tests you'll see a couple of issues:
- https://github.com/ansible-collections/community.aws/actions/runs/12383415807job/34597832812?pr=2205
- https://github.com/ansible-collections/community.aws/actions/runs/12383415807/job/34597834846?pr=2205
These boil down to
- The typo on line 63 breaks the documentation (YAML)
- We use the "black" formatting style, If you've got tox setup, then
tox -m formatit should fix the formatting.
WRT Unit tests:
Unit tests can be a PITA to implement (they live under tests/unit/plugins/modules/).
I'd really like to see some basic tests for filters_are_equal if possible. However, by moving filters_are_equal outside of compare_and_update_configuration you'll make it easier for someone (possibly me) to come along later and add them if you're not able to do so.
plugins/modules/s3_lifecycle.py
Outdated
| @@ -59,6 +59,16 @@ | |||
| and replaced with the new transition(s) | |||
| default: true | |||
| type: bool | |||
| maximum_object_size: | |||
| descriptionL | |||
There was a problem hiding this comment.
| descriptionL | |
| description: |
There was a problem hiding this comment.
@tremble - I have made the changes based on your comments. But still seeing the build on collections docs failing - https://github.com/ansible-collections/community.aws/actions/runs/12403295884
I have created a unit test but having trouble trying to run tests locally. Can this test be executed as part of CI?
There was a problem hiding this comment.
If you add it to tests/unit/plugins/modules/test_s3_lifecycle.py it'll be run automatically by CI. In some cases we need to manually approve running the tests which is why you'll sometimes see a delay.
I wanted to respond a couple of days ago and tell you it should be possible to run the tests via tox, only to realise that an updated python library somewhere caused some breakage. (#2206 should fix this)
Normally it should be possible to trigger the unit tests with
tox -m unit -- tests/unit/plugins/modules/
By default this triggers tests with a variety of Ansible and Python versions.
2. Updated code based on the feedback from PR review
|
Build succeeded. ❌ ansible-galaxy-importer FAILURE in 5m 44s (non-voting) |
plugins/modules/s3_lifecycle.py
Outdated
| maximum_object_size=dict(type=int), | ||
| minimum_object_size=dict(type=int), |
There was a problem hiding this comment.
| maximum_object_size=dict(type=int), | |
| minimum_object_size=dict(type=int), | |
| maximum_object_size=dict(type="int"), | |
| minimum_object_size=dict(type="int"), |
|
One other minor request: Please add a "minor_changes" changelog entry: https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs-how-to |
2. Included a changelog minor change fragment file. 3. Enclosed types within double quotes.
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 13s (non-voting) |
SUMMARY
Support the S3 lifecycle settings of minimum and maximum object size to apply the lifecycle rules.
Fixes #861
ISSUE TYPE
COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION