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.

Sign up for GitHub

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

Jump to bottom

connection/aws_ssm - refactor exec_command function to improve maintanability #2226

Merged
merged 2 commits into from
Feb 3, 2025
Merged

connection/aws_ssm - refactor exec_command function to improve maintanability #2226

merged 2 commits into from
Feb 3, 2025

Conversation

abikouo
Copy link
Contributor

@abikouo abikouo commented Jan 30, 2025

SUMMARY

Refer to https://issues.redhat.com/browse/ACA-2093
Refactor exec_command() and add unit tests

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

connection/aws_ssm

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

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

❌ ansible-galaxy-importer FAILURE in 4m 17s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 17s
✔️ ansible-test-splitter SUCCESS in 4m 06s
✔️ integration-community.aws-1 SUCCESS in 22m 17s
✔️ integration-community.aws-2 SUCCESS in 14m 36s
✔️ integration-community.aws-3 SUCCESS in 14m 21s
✔️ integration-community.aws-4 SUCCESS in 15m 29s
✔️ integration-community.aws-5 SUCCESS in 23m 32s
✔️ integration-community.aws-6 SUCCESS in 15m 09s
✔️ integration-community.aws-7 SUCCESS in 16m 05s
✔️ integration-community.aws-8 SUCCESS in 15m 42s
✔️ integration-community.aws-9 SUCCESS in 14m 09s
✔️ integration-community.aws-10 SUCCESS in 5m 30s
✔️ integration-community.aws-11 SUCCESS in 14m 53s
Skipped 11 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

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

❌ ansible-galaxy-importer FAILURE in 5m 54s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 51s
✔️ ansible-test-splitter SUCCESS in 4m 11s
✔️ integration-community.aws-1 SUCCESS in 24m 26s
✔️ integration-community.aws-2 SUCCESS in 13m 30s
✔️ integration-community.aws-3 SUCCESS in 15m 47s
✔️ integration-community.aws-4 SUCCESS in 14m 38s
✔️ integration-community.aws-5 SUCCESS in 24m 57s
✔️ integration-community.aws-6 SUCCESS in 14m 34s
✔️ integration-community.aws-7 SUCCESS in 14m 55s
✔️ integration-community.aws-8 SUCCESS in 14m 06s
✔️ integration-community.aws-9 SUCCESS in 17m 23s
✔️ integration-community.aws-10 SUCCESS in 5m 45s
✔️ integration-community.aws-11 SUCCESS in 16m 52s
Skipped 11 jobs

@abikouo
connection/aws_ssm - refactor exec_command function to improve readab…
a670fac 
…ility

fix linters

create dedicated conftest.py for aws_ssm

add conftest.py

fix isort
@abikouo abikouo force-pushed the ssm_exec_command_refactor branch from 24a2c7d to a670fac Compare January 30, 2025 16:49
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

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

❌ ansible-galaxy-importer FAILURE in 5m 49s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 38s
✔️ ansible-test-splitter SUCCESS in 3m 54s
✔️ integration-community.aws-1 SUCCESS in 24m 52s
✔️ integration-community.aws-2 SUCCESS in 15m 28s
✔️ integration-community.aws-3 SUCCESS in 17m 15s
✔️ integration-community.aws-4 SUCCESS in 15m 42s
✔️ integration-community.aws-5 SUCCESS in 14m 38s
✔️ integration-community.aws-6 SUCCESS in 15m 31s
✔️ integration-community.aws-7 SUCCESS in 15m 05s
✔️ integration-community.aws-8 SUCCESS in 14m 59s
✔️ integration-community.aws-9 SUCCESS in 15m 27s
✔️ integration-community.aws-10 SUCCESS in 5m 46s
✔️ integration-community.aws-11 SUCCESS in 15m 21s
Skipped 11 jobs

GomathiselviS
GomathiselviS reviewed Jan 31, 2025
View reviewed changes
plugins/connection/aws_ssm.py Show resolved Hide resolved
plugins/connection/aws_ssm.py Show resolved Hide resolved
plugins/connection/aws_ssm.py Outdated Show resolved Hide resolved
plugins/connection/aws_ssm.py Outdated Show resolved Hide resolved
@abikouo abikouo requested a review from GomathiselviS February 3, 2025 11:44
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

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

✔️ ansible-galaxy-importer SUCCESS in 3m 45s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 32s
✔️ ansible-test-splitter SUCCESS in 3m 59s
✔️ integration-community.aws-1 SUCCESS in 21m 23s
✔️ integration-community.aws-2 SUCCESS in 14m 53s
✔️ integration-community.aws-3 SUCCESS in 14m 49s
✔️ integration-community.aws-4 SUCCESS in 16m 28s
✔️ integration-community.aws-5 SUCCESS in 22m 49s
✔️ integration-community.aws-6 SUCCESS in 14m 42s
✔️ integration-community.aws-7 SUCCESS in 15m 05s
✔️ integration-community.aws-8 SUCCESS in 16m 54s
✔️ integration-community.aws-9 SUCCESS in 14m 58s
✔️ integration-community.aws-10 SUCCESS in 6m 26s
✔️ integration-community.aws-11 SUCCESS in 14m 49s
Skipped 11 jobs

tremble
tremble reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Is this batch of changes ready? Only thing that's standing out to me right now is the pylint stuff

plugins/connection/aws_ssm.py Show resolved Hide resolved
plugins/connection/aws_ssm.py Show resolved Hide resolved
@abikouo
Copy link
Contributor Author

abikouo commented Feb 3, 2025

Is this batch of changes ready? Only thing that's standing out to me right now is the pylint stuff

Code is ready for review/approval. Refactoring of _prepare_terminal() method is almost ready and depending on this one

@abikouo abikouo requested a review from tremble February 3, 2025 16:02
@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Feb 3, 2025
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

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

❌ ansible-galaxy-importer FAILURE in 4m 27s (non-voting)
✔️ build-ansible-collection SUCCESS in 10m 17s
✔️ ansible-test-splitter SUCCESS in 3m 52s
✔️ integration-community.aws-1 SUCCESS in 24m 07s
✔️ integration-community.aws-2 SUCCESS in 14m 39s
✔️ integration-community.aws-3 SUCCESS in 16m 02s
✔️ integration-community.aws-4 SUCCESS in 15m 26s
✔️ integration-community.aws-5 SUCCESS in 14m 45s
✔️ integration-community.aws-6 SUCCESS in 16m 01s
✔️ integration-community.aws-7 SUCCESS in 14m 53s
✔️ integration-community.aws-8 SUCCESS in 17m 05s
✔️ integration-community.aws-9 SUCCESS in 14m 12s
✔️ integration-community.aws-10 SUCCESS in 4m 16s
✔️ integration-community.aws-11 SUCCESS in 15m 08s
Skipped 11 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit d84f3a1 into ansible-collections:main Feb 3, 2025
86 of 87 checks passed
@patchback Patchback
Copy link

patchback bot commented Feb 3, 2025 •
edited
Loading

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

❌ Failed to cleanly apply d84f3a1 on top of patchback/backports/stable-9/d84f3a1635763fffd6d3a91fe765beb0c7d000d9/pr-2226

Backporting merged PR #2226 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/d84f3a1635763fffd6d3a91fe765beb0c7d000d9/pr-2226 upstream/stable-9
  4. Now, cherry-pick PR connection/aws_ssm - refactor exec_command function to improve maintanability #2226 contents into that branch: 
    $ git cherry-pick -x d84f3a1635763fffd6d3a91fe765beb0c7d000d9
    If it'll yell at you with something like fatal: Commit d84f3a1635763fffd6d3a91fe765beb0c7d000d9 is a merge but no -m option was given., add -m 1 as follows instead: 
    $ git cherry-pick -m1 -x d84f3a1635763fffd6d3a91fe765beb0c7d000d9
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR connection/aws_ssm - refactor exec_command function to improve maintanability #2226 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub: 
    $ git push origin patchback/backports/stable-9/d84f3a1635763fffd6d3a91fe765beb0c7d000d9/pr-2226
  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.

@patchback Patchback
Copy link

patchback bot commented Feb 4, 2025 •
edited
Loading

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/d84f3a1635763fffd6d3a91fe765beb0c7d000d9/pr-2226

Backported as #2231

🤖 @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 Feb 4, 2025
@abikouo @patchback
connection/aws_ssm - refactor exec_command function to improve mainta…
161d530 
…nability (#2226)

SUMMARY

Refer to https://issues.redhat.com/browse/ACA-2093
Refactor exec_command() and add unit tests

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

connection/aws_ssm

Reviewed-by: GomathiselviS <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Mark Chappell
(cherry picked from commit d84f3a1)
@patchback patchback bot mentioned this pull request Feb 4, 2025
Merged
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 4, 2025
@patchback
connection/aws_ssm - refactor exec_command function to improve mainta…
b7690aa 
…nability (#2226) (#2231)

This is a backport of PR #2226 as merged into main (d84f3a1).
SUMMARY

Refer to https://issues.redhat.com/browse/ACA-2093
Refactor exec_command() and add unit tests

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
Reviewers

@tremble tremble tremble approved these changes

@GomathiselviS GomathiselviS GomathiselviS approved these changes

@alinabuzachis alinabuzachis Awaiting requested review from alinabuzachis

Assignees
No one assigned
Labels
backport_failed backport-9 mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abikouo @tremble @GomathiselviS