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

aws_s3: implement COPY object on a bucket #30741

Open
wants to merge 1 commit into
base: devel
from

Conversation

@chtitux
Copy link

commented Sep 22, 2017

SUMMARY

Implement the COPY action on a S3 bucket

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

aws_s3

ANSIBLE VERSION

Devlopment

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2017

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2017

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:384:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/amazon/aws_s3.py:581:14: E222 multiple spaces after operator

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/cloud/amazon/aws_s3.py:0:0: E309 version_added for new option (src_obj) should be 2.5. Currently 0.0

click here for bot help

@chtitux chtitux force-pushed the chtitux:aws-s3-copy-object branch 3 times, most recently Sep 22, 2017

@ansibot ansibot removed the ci_verified label Sep 22, 2017

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2017

The test ansible-test sanity --test pep8 [?] failed with the following error:

lib/ansible/modules/cloud/amazon/aws_s3.py:399:161: E501 line too long (208 > 160 characters)

click here for bot help

@chtitux chtitux force-pushed the chtitux:aws-s3-copy-object branch to 928cfbe Sep 22, 2017

@ansibot ansibot added the stale_ci label Sep 30, 2017

@ansibot ansibot removed the new_contributor label Nov 3, 2017

@ansibot ansibot added needs_rebase and removed core_review labels Dec 21, 2017

@s-hertel
Copy link
Contributor

left a comment

This seems like a good thing to have. I wonder if this is overloading this module (it already seems overloaded, since it handles both buckets and objects). Would this be better as its own module? New features to this module definitely needs integration tests as it is stableinterface.

# Copy an object from a bucket, not the entire bucket
if mode == 'copyobj':
if obj is None:
module.fail_json(msg="object parameter is required")

This comment has been minimized.

Copy link
@s-hertel

s-hertel Apr 26, 2019

Contributor

Rather than doing validation like this here, you should be able to add a required_if option to AnsibleModule:

    module = AnsibleModule(
        argument_spec=argument_spec,
        supports_check_mode=True,
        required_if=[('mode', 'copyobj', ('obj',))]
    )
src_bucket = bucket
if bucket:
copyrtn = copy_key(module, s3, bucket, src_bucket, obj, src_obj)
if copyrtn is True:

This comment has been minimized.

Copy link
@s-hertel

s-hertel Apr 26, 2019

Contributor

It looks like copy_key calls exit_json so you don't need this line and the one following.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.