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

Refactor aws_ssm display and verbosity-related methods #2264

Conversation

beeankha
Copy link
Contributor

@beeankha beeankha commented Mar 7, 2025

SUMMARY

Resolves ACA-2239

Methods like self._vvvv, self._vvv, etc. have been refactored into a single method, including code from the _display() method.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

connection/aws_ssm

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/92c1b576a24b4e3c9140a29bfb04e919

ansible-galaxy-importer FAILURE in 5m 28s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 31s
✔️ ansible-test-splitter SUCCESS in 3m 56s
✔️ integration-community.aws-1 SUCCESS in 24m 03s
✔️ integration-community.aws-2 SUCCESS in 15m 50s
✔️ integration-community.aws-3 SUCCESS in 15m 49s
✔️ integration-community.aws-4 SUCCESS in 14m 12s
✔️ integration-community.aws-5 SUCCESS in 14m 00s
✔️ integration-community.aws-6 SUCCESS in 14m 43s
✔️ integration-community.aws-7 SUCCESS in 14m 06s
✔️ integration-community.aws-8 SUCCESS in 13m 46s
✔️ integration-community.aws-9 SUCCESS in 15m 16s
✔️ integration-community.aws-10 SUCCESS in 6m 15s
✔️ integration-community.aws-11 SUCCESS in 16m 09s
Skipped 11 jobs

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/3fc99c1ffcb84be6a9e5a8edfe4afe5f

ansible-galaxy-importer FAILURE in 4m 04s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 28s
✔️ ansible-test-splitter SUCCESS in 4m 00s
✔️ integration-community.aws-1 SUCCESS in 21m 47s
✔️ integration-community.aws-2 SUCCESS in 16m 30s
✔️ integration-community.aws-3 SUCCESS in 15m 44s
✔️ integration-community.aws-4 SUCCESS in 14m 49s
✔️ integration-community.aws-5 SUCCESS in 14m 36s
✔️ integration-community.aws-6 SUCCESS in 14m 31s
✔️ integration-community.aws-7 SUCCESS in 14m 59s
✔️ integration-community.aws-8 SUCCESS in 14m 11s
integration-community.aws-9 RETRY_LIMIT in 1m 51s
✔️ integration-community.aws-10 SUCCESS in 5m 27s
✔️ integration-community.aws-11 SUCCESS in 16m 02s
Skipped 11 jobs

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/b99a1dfe85644ced92f8f81adcda7af3

ansible-galaxy-importer FAILURE in 4m 29s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 24s
✔️ ansible-test-splitter SUCCESS in 4m 24s
✔️ integration-community.aws-1 SUCCESS in 22m 31s
✔️ integration-community.aws-2 SUCCESS in 17m 09s
✔️ integration-community.aws-3 SUCCESS in 15m 21s
✔️ integration-community.aws-4 SUCCESS in 15m 52s
✔️ integration-community.aws-5 SUCCESS in 14m 25s
✔️ integration-community.aws-6 SUCCESS in 14m 26s
✔️ integration-community.aws-7 SUCCESS in 16m 36s
✔️ integration-community.aws-8 SUCCESS in 16m 02s
✔️ integration-community.aws-9 SUCCESS in 13m 39s
✔️ integration-community.aws-10 SUCCESS in 4m 44s
✔️ integration-community.aws-11 SUCCESS in 15m 41s
Skipped 11 jobs

@beeankha beeankha force-pushed the refactor_aws_ssm_display_verbosity branch from f247096 to 12b3fde Compare March 10, 2025 21:38
@beeankha beeankha force-pushed the refactor_aws_ssm_display_verbosity branch from 12b3fde to eda3496 Compare March 10, 2025 21:40
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/5e1ac7467f864ff7982aa49b577edc10

✔️ ansible-galaxy-importer SUCCESS in 3m 06s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 23s
✔️ ansible-test-splitter SUCCESS in 5m 03s
✔️ integration-community.aws-1 SUCCESS in 22m 32s
✔️ integration-community.aws-2 SUCCESS in 14m 14s
✔️ integration-community.aws-3 SUCCESS in 19m 26s
✔️ integration-community.aws-4 SUCCESS in 14m 15s
✔️ integration-community.aws-5 SUCCESS in 14m 34s
✔️ integration-community.aws-6 SUCCESS in 14m 30s
✔️ integration-community.aws-7 SUCCESS in 14m 36s
integration-community.aws-8 FAILURE in 7m 39s
✔️ integration-community.aws-9 SUCCESS in 14m 49s
✔️ integration-community.aws-10 SUCCESS in 4m 49s
✔️ integration-community.aws-11 SUCCESS in 13m 02s
Skipped 11 jobs

next(poll_gen)

# Verify verbosity message contains remaining time
mock_display.assert_called_with(4, "TEST remaining: 10 second(s)")
Copy link
Contributor Author

@beeankha beeankha Mar 10, 2025

Choose a reason for hiding this comment

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

I figured it might be a good way to test verbosity_display() by utilizing a method that calls verbosity_display(), but maybe this is gonna cause more trouble than it's worth since the poll() method might move in the future?

@beeankha beeankha marked this pull request as ready for review March 10, 2025 23:10
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/706d77d0a86f4913b1b1fca55c9517b9

ansible-galaxy-importer FAILURE in 5m 03s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 18s
✔️ ansible-test-splitter SUCCESS in 4m 04s
✔️ integration-community.aws-1 SUCCESS in 24m 52s
✔️ integration-community.aws-2 SUCCESS in 13m 26s
✔️ integration-community.aws-3 SUCCESS in 15m 21s
✔️ integration-community.aws-4 SUCCESS in 15m 35s
✔️ integration-community.aws-5 SUCCESS in 14m 09s
✔️ integration-community.aws-6 SUCCESS in 15m 08s
✔️ integration-community.aws-7 SUCCESS in 14m 46s
✔️ integration-community.aws-8 SUCCESS in 14m 11s
✔️ integration-community.aws-9 SUCCESS in 15m 23s
✔️ integration-community.aws-10 SUCCESS in 7m 15s
✔️ integration-community.aws-11 SUCCESS in 14m 43s
Skipped 11 jobs

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/cffc2de952f540c0ae9d4239481f52b4

ansible-galaxy-importer FAILURE in 8m 12s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 27s
✔️ ansible-test-splitter SUCCESS in 4m 06s
✔️ integration-community.aws-1 SUCCESS in 23m 06s
✔️ integration-community.aws-2 SUCCESS in 15m 36s
✔️ integration-community.aws-3 SUCCESS in 16m 14s
✔️ integration-community.aws-4 SUCCESS in 17m 09s
✔️ integration-community.aws-5 SUCCESS in 15m 11s
✔️ integration-community.aws-6 SUCCESS in 16m 13s
✔️ integration-community.aws-7 SUCCESS in 15m 38s
✔️ integration-community.aws-8 SUCCESS in 14m 44s
✔️ integration-community.aws-9 SUCCESS in 15m 42s
✔️ integration-community.aws-10 SUCCESS in 8m 29s
✔️ integration-community.aws-11 SUCCESS in 16m 18s
Skipped 11 jobs

self._client = self._get_boto_client(
"ssm",
region_name=region_name,
profile_name=profile_name,
)

def _display(self, f: Any, message: str) -> None:
def verbosity_display(self, level: int, message: str) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

The function docs say there's a default value of 1, but that's not represented in the function signature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I meant to remove that default value from the docstring.

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/14160ab94e204a4d9a158a6fc0c6b19a

ansible-galaxy-importer FAILURE in 5m 03s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 21s
✔️ ansible-test-splitter SUCCESS in 4m 44s
✔️ integration-community.aws-1 SUCCESS in 23m 17s
✔️ integration-community.aws-2 SUCCESS in 14m 37s
✔️ integration-community.aws-3 SUCCESS in 15m 04s
✔️ integration-community.aws-4 SUCCESS in 15m 46s
✔️ integration-community.aws-5 SUCCESS in 15m 07s
✔️ integration-community.aws-6 SUCCESS in 16m 38s
✔️ integration-community.aws-7 SUCCESS in 14m 50s
✔️ integration-community.aws-8 SUCCESS in 14m 42s
✔️ integration-community.aws-9 SUCCESS in 14m 24s
✔️ integration-community.aws-10 SUCCESS in 5m 42s
✔️ integration-community.aws-11 SUCCESS in 16m 36s
Skipped 11 jobs

@beeankha beeankha added the mergeit Merge the PR (SoftwareFactory) label Mar 13, 2025
@beeankha beeankha self-assigned this Mar 13, 2025
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/2a06445a6bf349f2a274417a8b11ef3e

ansible-galaxy-importer FAILURE in 4m 56s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 25s
✔️ ansible-test-splitter SUCCESS in 4m 06s
✔️ integration-community.aws-1 SUCCESS in 23m 01s
✔️ integration-community.aws-2 SUCCESS in 15m 33s
✔️ integration-community.aws-3 SUCCESS in 16m 42s
✔️ integration-community.aws-4 SUCCESS in 15m 21s
✔️ integration-community.aws-5 SUCCESS in 14m 08s
✔️ integration-community.aws-6 SUCCESS in 15m 33s
✔️ integration-community.aws-7 SUCCESS in 15m 16s
✔️ integration-community.aws-8 SUCCESS in 14m 16s
✔️ integration-community.aws-9 SUCCESS in 15m 11s
✔️ integration-community.aws-10 SUCCESS in 4m 42s
✔️ integration-community.aws-11 SUCCESS in 15m 10s
Skipped 11 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit a9a900a into ansible-collections:main Mar 13, 2025
86 of 87 checks passed
Copy link

patchback bot commented Mar 13, 2025

Backport to stable-9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply a9a900a on top of patchback/backports/stable-9/a9a900a3d42d2c65b10d232d4387fec42174fb25/pr-2264

Backporting merged PR #2264 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/community.aws.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-9/a9a900a3d42d2c65b10d232d4387fec42174fb25/pr-2264 upstream/stable-9
  4. Now, cherry-pick PR Refactor aws_ssm display and verbosity-related methods #2264 contents into that branch:
    $ git cherry-pick -x a9a900a3d42d2c65b10d232d4387fec42174fb25
    If it'll yell at you with something like fatal: Commit a9a900a3d42d2c65b10d232d4387fec42174fb25 is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x a9a900a3d42d2c65b10d232d4387fec42174fb25
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Refactor aws_ssm display and verbosity-related methods #2264 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-9/a9a900a3d42d2c65b10d232d4387fec42174fb25/pr-2264
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

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

beeankha added a commit to beeankha/community.aws that referenced this pull request Mar 13, 2025
…lections#2264)

SUMMARY
Resolves ACA-2239
Methods like self._vvvv, self._vvv, etc. have been refactored into a single method, including code from the _display() method.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

connection/aws_ssm

Reviewed-by: Bianca Henderson <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Mike Graves <[email protected]>
Reviewed-by: Rahmanim Benny <[email protected]>
(cherry picked from commit a9a900a)
@beeankha
Copy link
Contributor Author

Backporting of this PR requires #2266 (a backport PR) to be merged first.

Copy link

patchback bot commented Mar 20, 2025

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/a9a900a3d42d2c65b10d232d4387fec42174fb25/pr-2264

Backported as #2269

🤖 @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 Mar 20, 2025
SUMMARY
Resolves ACA-2239
Methods like self._vvvv, self._vvv, etc. have been refactored into a single method, including code from the _display() method.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME

connection/aws_ssm

Reviewed-by: Bianca Henderson <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Mike Graves <[email protected]>
Reviewed-by: Rahmanim Benny <[email protected]>
(cherry picked from commit a9a900a)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Mar 21, 2025
This is a backport of PR #2264 as merged into main (a9a900a).
SUMMARY
Resolves ACA-2239
Methods like self._vvvv, self._vvv, etc. have been refactored into a single method, including code from the _display() method.
ISSUE TYPE


Feature Pull Request

COMPONENT NAME

connection/aws_ssm

Reviewed-by: Bikouo Aubin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport_failed backport-9 mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants