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

Feature: mode listextended returns object attributes (48624) #58542

Open
wants to merge 3 commits into
base: devel
from

Conversation

@groorj
Copy link

commented Jun 29, 2019

SUMMARY

This PR implements a solution to return all the object attributes and not only LastModified as it was suggested. Created a new mode named listextended and kept the original list mode for backward compatibility.

Fixes #48624

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

aws_s3

ADDITIONAL INFORMATION

Here is the original return from list:

ok: [localhost] => {
    "aws_output": {
        "changed": false,
        "failed": false,
        "msg": "LIST operation complete",
        "s3_keys": [
            "file1.csv",
            "file2.csv.gz"
        ]
    }
}

Here is the return using the new mode listextended:

ok: [localhost] => {
    "aws_output": {
        "changed": false,
        "failed": false,
        "msg": "LIST operation complete",
        "s3_keys": [
            {
                "ETag": "\"a7b7d35bf8178f30acef9f8b2adf6402\"",
                "Key": “file1.csv",
                "LastModified": "2018-09-14T00:39:32+00:00",
                "Size": 12836751,
                "StorageClass": "REDUCED_REDUNDANCY"
            },
            {
                "ETag": "\"9a6f69ad00c974c0371272e178a37ffe\"",
                "Key": “file2.csv.gz",
                "LastModified": "2018-09-14T00:26:51+00:00",
                "Size": 1978,
                "StorageClass": "REDUCED_REDUNDANCY"
            }
        ]
    }
}

This is my first PR here so feedback is welcomed.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

@groorj this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

@groorj, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_s3.py:456:11: singleton-comparison Comparison to True should be just 'expr'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:88:161: E501 line too long (182 > 160 characters)
lib/ansible/modules/cloud/amazon/aws_s3.py:343:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/amazon/aws_s3.py:456:26: E712 comparison to True should be 'if cond is True:' or 'if cond:'

The test ansible-test sanity --test yamllint [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:303:1: key-duplicates RETURN: duplication of key "s3_keys" in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:314:17: key-duplicates RETURN: duplication of key "..." in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:321:17: key-duplicates RETURN: duplication of key "..." in mapping

click here for bot help

@groorj groorj force-pushed the groorj:devel branch from d49e542 to 1700df4 Jun 29, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_s3.py:456:11: singleton-comparison Comparison to True should be just 'expr'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:88:161: E501 line too long (182 > 160 characters)
lib/ansible/modules/cloud/amazon/aws_s3.py:343:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/amazon/aws_s3.py:456:26: E712 comparison to True should be 'if cond is True:' or 'if cond:'

The test ansible-test sanity --test yamllint [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:303:1: key-duplicates RETURN: duplication of key "s3_keys" in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:314:17: key-duplicates RETURN: duplication of key "..." in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:321:17: key-duplicates RETURN: duplication of key "..." in mapping

click here for bot help

@ansibot ansibot added the ci_verified label Jun 29, 2019

@groorj groorj force-pushed the groorj:devel branch from 1700df4 to 9cf9c8d Jul 1, 2019

@ansibot ansibot removed the ci_verified label Jul 1, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_s3.py:456:11: singleton-comparison Comparison to True should be just 'expr'

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:88:161: E501 line too long (182 > 160 characters)
lib/ansible/modules/cloud/amazon/aws_s3.py:343:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/amazon/aws_s3.py:456:26: E712 comparison to True should be 'if cond is True:' or 'if cond:'

The test ansible-test sanity --test yamllint [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:303:1: key-duplicates RETURN: duplication of key "s3_keys" in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:314:17: key-duplicates RETURN: duplication of key "..." in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:321:17: key-duplicates RETURN: duplication of key "..." in mapping

click here for bot help

@jillr
Copy link
Contributor

left a comment

Thanks for this patch @groorj!
Could you also please update the integration tests at test/integration/targets/aws_s3/tasks/main.yml to cover this new feature?

Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_s3.py Outdated
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_s3.py Outdated
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_s3.py Outdated
@groorj

This comment has been minimized.

Copy link
Author

commented Jul 5, 2019

Thanks for the feedback @jillr. I will review it soon and push it again.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 6, 2019

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:88:161: E501 line too long (182 > 160 characters)
lib/ansible/modules/cloud/amazon/aws_s3.py:342:1: E302 expected 2 blank lines, found 1

The test ansible-test sanity --test yamllint [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:303:1: key-duplicates RETURN: duplication of key "s3_keys" in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:314:17: key-duplicates RETURN: duplication of key "..." in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:321:17: key-duplicates RETURN: duplication of key "..." in mapping

click here for bot help

@ansibot ansibot removed the ci_verified label Jul 6, 2019

@jillr
Copy link
Contributor

left a comment

This looks pretty good once the sanity tests are passing, just one small thing needed to make the aws tests pass.

Show resolved Hide resolved test/integration/targets/aws_s3/tasks/main.yml Outdated
Update test/integration/targets/aws_s3/tasks/main.yml
Fixing test task.

Co-Authored-By: Jill R <4121322+jillr@users.noreply.github.com>
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:88:161: E501 line too long (182 > 160 characters)
lib/ansible/modules/cloud/amazon/aws_s3.py:342:1: E302 expected 2 blank lines, found 1

The test ansible-test sanity --test yamllint [explain] failed with 3 errors:

lib/ansible/modules/cloud/amazon/aws_s3.py:303:1: key-duplicates RETURN: duplication of key "s3_keys" in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:314:17: key-duplicates RETURN: duplication of key "..." in mapping
lib/ansible/modules/cloud/amazon/aws_s3.py:321:17: key-duplicates RETURN: duplication of key "..." in mapping

click here for bot help

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