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

Add abort multipart upload and expire obj del markers to s3 lifecycle #794

Merged

Conversation

mdavis-xyz
Copy link
Contributor

@mdavis-xyz mdavis-xyz commented Nov 8, 2021

Depends-On: ansible/ansible-zuul-jobs#1247

SUMMARY

Fixes #365 #796

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

s3_lifecycle

ADDITIONAL INFORMATION

I have not run integration tests yet because of ansible-collections/amazon.aws#924.

I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

@mdavis-xyz mdavis-xyz force-pushed the s3_lifecycle_abort_expire branch from eee8b44 to 5fe4863 Compare November 9, 2021 23:11
@mdavis-xyz
Copy link
Contributor Author

Why does the CICD say

ansible/gate Expected — Waiting for status to be reported

What does that mean?

@markuman
Copy link
Member

Why does the CICD say

ansible/gate Expected — Waiting for status to be reported

What does that mean?

It's just waiting for at least two reviewers/approvals.

@tremble
Copy link
Contributor

tremble commented Nov 11, 2021

What does that mean?

It means that the conditions for the "gate" job hasn't been met, so the job hasn't been allowed to run. It's based on the work that the OpenStack folks have done https://dague.net/2013/02/21/the-openstack-gate/

Once triggered (and successful) Zuul will automatically merge the change. Nominally it's a subset of the 'ansible/check' tests and it's used to ensure that between ansible/check having been run and the change being approved another change hasn't been introduced that clashes with this one (for example a function the change depended on being modified in an incompatible manner)

Copy link
Contributor

@alinabuzachis alinabuzachis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdavis-xyz Thank you for working on this. Generally this LGTM! Could you also please add a changelog fragment?

plugins/modules/s3_lifecycle.py Show resolved Hide resolved
plugins/modules/s3_lifecycle.py Outdated Show resolved Hide resolved
plugins/modules/s3_lifecycle.py Outdated Show resolved Hide resolved
plugins/modules/s3_lifecycle.py Outdated Show resolved Hide resolved
plugins/modules/s3_lifecycle.py Outdated Show resolved Hide resolved
@goneri
Copy link
Member

goneri commented Nov 30, 2021

check ondemand

1 similar comment
@goneri
Copy link
Member

goneri commented Nov 30, 2021

check ondemand

@mdavis-xyz
Copy link
Contributor Author

I've added the changelog, and changed C() to I(). (Although I don't know what the difference between C and I is)

@tremble
Copy link
Contributor

tremble commented Dec 1, 2021

https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#linking-and-other-format-macros-within-module-documentation

There are also some macros which do not create links but we use them to display certain types of content in a uniform way:

    I() for option names. For example: Required if I(state=present). This is italicized in the documentation.

    C() for files, option values, and inline code. For example: If not set the environment variable C(ACME_PASSWORD) will be used. or Use C(var | foo.bar.my_filter) to transform C(var) into the required format. This displays with a mono-space font in the documentation.

@tremble tremble added backport-2 PR should be backported to the stable-2 branch gate labels Dec 1, 2021
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tremble tremble requested a review from alinabuzachis December 1, 2021 12:33
@ansible-zuul ansible-zuul bot removed the gate label Dec 1, 2021
@markuman markuman added the gate label Dec 1, 2021
@tremble tremble dismissed alinabuzachis’s stale review December 2, 2021 08:31

issues addressed

@tremble tremble removed the gate label Dec 2, 2021
@tremble
Copy link
Contributor

tremble commented Dec 2, 2021

recheck

@mdavis-xyz
Copy link
Contributor Author

mdavis-xyz commented Dec 22, 2021

Can someone please open the gate for me?

@markuman markuman added the gate label Jan 14, 2022
@ansible-zuul ansible-zuul bot merged commit ed3a7a0 into ansible-collections:main Jan 14, 2022
@patchback
Copy link

patchback bot commented Jan 14, 2022

Backport to stable-2: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-2/ed3a7a0b0ae86cbcaed733ada1f88a27917e1a09/pr-794

Backported as #864

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 14, 2022
…#794)

Add abort multipart upload and expire obj del markers to s3 lifecycle

Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes #365 #796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of #793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Matthew Davis <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: None <None>
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit ed3a7a0)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jan 17, 2022
…#794) (#864)

[PR #794/ed3a7a0b backport][stable-2] Add abort multipart upload and expire obj del markers to s3 lifecycle

This is a backport of PR #794 as merged into main (ed3a7a0).
Depends-On: ansible/ansible-zuul-jobs#1247
SUMMARY
Fixes #365 #796
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_lifecycle
ADDITIONAL INFORMATION
I have not run integration tests yet because of #793.
I'm unsure about how to name and structure the new arguments.
Do I nest them to match the API, or flatten them to match existing arguments?
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
Updates to the Developer Guidelines

SUMMARY
Updates for the Developer Guidelines (originally started as ansible/ansible#77598)
ISSUE TYPE

Docs Pull Request

COMPONENT NAME
docs/docsite/rst/dev_guidelines.rst
ADDITIONAL INFORMATION
Validation run can be found at:
https://github.com/tremble/amazon.aws/runs/6192239999?check_suite_focus=true

Reviewed-by: Felix Fontein <[email protected]>
Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-2 PR should be backported to the stable-2 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants