refactor connection/aws_ssm plugin #2275
Conversation
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 11s (non-voting) |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 26s (non-voting) |
|
Build succeeded. ❌ ansible-galaxy-importer FAILURE in 5m 57s (non-voting) |
plugins/plugin_utils/base.py
Outdated
| region_name: Optional[str] = None, | ||
| endpoint_url: Optional[str] = None, | ||
| config: Optional[Dict[str, Any]] = None, | ||
| ): |
There was a problem hiding this comment.
Should the return type be BaseClient ?
There was a problem hiding this comment.
The botocore does use type hint, it is hard to deduced so putting Any as returned value is enough
| command = None | ||
| url = None | ||
| put_args = None | ||
| if method == "get": |
There was a problem hiding this comment.
Is this right? I am little confused as you are using put_args and "PUT" method under this. Would you mind using the correct variable names and method values to avoid the confusion?
There was a problem hiding this comment.
Yes, this is confusing at first glance but it's correct (I was also puzzled by this when working on #2248). @abikouo please correct me if I'm wrong but from my understanding, the reason for this apparent contradiction is that the code is handling file transfers between a local machine and a remote AWS instance via S3 as an intermediary. The ssm_action refers to what the user/caller wants to do (get a file from the remote instance or put a file to the remote instance), while the method in the commands refers to what the remote instance needs to do.
@GomathiselviS @abikouo do you think it might be helpful to add some comments to clarify why the specified methods seem to be the opposite of what they should be?
There was a problem hiding this comment.
Thanks for the clarification, @beeankha. Yes, comments would be helpful for posterity.
There was a problem hiding this comment.
@beeankha, you are right!! The file transfer generates 2 commands, one for the controller node and another for the managed node. The controller node uses the AWS S3 API to download/put file to S3 bucket while the managed node uses the curl command to download/upload file to bucket.
There was a problem hiding this comment.
The function description has been updated
caba9b3 to
242aae1
Compare
|
Build succeeded. ❌ ansible-galaxy-importer FAILURE in 4m 59s (non-voting) |
|
Build succeeded (gate pipeline). ❌ ansible-galaxy-importer FAILURE in 3m 47s (non-voting) |
b41876d
into
ansible-collections:main
Backport to stable-9: 💚 backport PR created✅ Backport PR branch: Backported as #2280 🤖 @patchback |
SUMMARY Refactor S3 operation function from connection/aws_ssm plugin Refer to: https://issues.redhat.com/browse/ACA-2100 ISSUE TYPE Feature Pull Request Reviewed-by: GomathiselviS <gomathiselvi@gmail.com> Reviewed-by: Bianca Henderson <beeankha@gmail.com> Reviewed-by: Bikouo Aubin (cherry picked from commit b41876d)
This is a backport of PR #2275 as merged into main (b41876d). SUMMARY Refactor S3 operation function from connection/aws_ssm plugin Refer to: https://issues.redhat.com/browse/ACA-2100 ISSUE TYPE Feature Pull Request Reviewed-by: Bikouo Aubin
SUMMARY Refactor S3 operation function from connection/aws_ssm plugin Refer to: https://issues.redhat.com/browse/ACA-2100 ISSUE TYPE Feature Pull Request Reviewed-by: GomathiselviS <gomathiselvi@gmail.com> Reviewed-by: Bianca Henderson <beeankha@gmail.com> Reviewed-by: Bikouo Aubin This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@b41876d
SUMMARY
Refactor S3 operation function from
connection/aws_ssmpluginRefer to: https://issues.redhat.com/browse/ACA-2100
ISSUE TYPE