s3_bucket: Refactor s3 bucket module #2057
Conversation
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 49s |
mandar242
force-pushed
the
refactor_s3_bucket_module
branch
from
d146ce9 to
002ea20
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 11s |
There was a problem hiding this comment.
Generally looking good, I'd actually started looking into refactoring s3_object with an eye towards using @AWSErrorHandler (see also #1998). Which would probably be a nice follow up to this one.
The use of hinting seems inconsistent, and some of the return documentation looks dubious (eg. get_bucket_versioning returning None).
mandar242
force-pushed
the
refactor_s3_bucket_module
branch
from
1da1311 to
b9320b5
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 29s |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 5m 18s |
plugins/modules/s3_bucket.py
Outdated
| @@ -352,6 +352,7 @@ | |||
|
|
|||
| import json | |||
| import time | |||
| from typing import Iterator, List, Tuple | |||
There was a problem hiding this comment.
| from typing import Iterator, List, Tuple | |
| from typing import Iterator | |
| from typing import List | |
| from typing import Tuple |
| requester_pays = module.params.get("requester_pays") | ||
| tags = module.params.get("tags") | ||
| purge_tags = module.params.get("purge_tags") | ||
| def handle_bucket_versioning(s3_client, module: AnsibleAWSModule, name: str) -> tuple[bool, dict]: |
There was a problem hiding this comment.
why not set the attribute type boto3.client for the s3_client as described in the function documentation?
There was a problem hiding this comment.
when added s3_client: boto3.client its throwing sanity failures and integration test failures (import)
ERROR: Found 47 pylint issue(s) which need to be resolved:
ERROR: plugins/modules/s3_bucket.py:378:40: undefined-variable: Undefined variable 'boto3'
ERROR: plugins/modules/s3_bucket.py:432:44: undefined-variable: Undefined variable 'boto3'
ERROR: plugins/modules/s3_bucket.py:476:50: undefined-variable: Undefined variable 'boto3'
one thing we can do is add import boto3 but I am not sure if doing that would cause boto version issues
|
|
||
| # Requester pays | ||
|
|
||
| def handle_bucket_requester_pays(s3_client, module: AnsibleAWSModule, name: str) -> tuple[bool, dict]: |
There was a problem hiding this comment.
same remark here for the s3_client parameter
mandar242
force-pushed
the
refactor_s3_bucket_module
branch
from
391c25d to
692ee36
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 06s |
|
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 5m 11s |
softwarefactory-project-zuul
bot
merged commit 99e13ae
into
ansible-collections:main
SUMMARY
Refactor s3_bucket module
ISSUE TYPE
COMPONENT NAME
s3_bucket
ADDITIONAL INFORMATION