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

Linting cleanup #1181

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Oct 18, 2022

SUMMARY

Minor linting cleanup

  • missing whitespace
  • unused variables
  • catching Exception
  • unused imports

in kms_key also replaces get_account_info with module_utils.iam.get_aws_account_info

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/inventory/aws_ec2.py
plugins/module_utils/cloud.py
plugins/modules/autoscaling_group.py
plugins/modules/cloudtrail_info.py
plugins/modules/cloudwatchlogs_log_group.py
plugins/modules/ec2_eip.py
plugins/modules/kms_key_info.py
plugins/modules/lambda.py
plugins/modules/lambda_execute.py
plugins/modules/rds_cluster_snapshot.py
plugins/modules/rds_instance.py
plugins/modules/rds_instance_snapshot.py
plugins/modules/route53_health_check.py
plugins/modules/s3_object_info.py

ADDITIONAL INFORMATION

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request inventory inventory plugin module module module_utils module_utils needs_maintainer needs_triage plugins plugin (any type) tests tests labels Oct 18, 2022
@tremble tremble force-pushed the sanity/2022-10-18 branch 2 times, most recently from bb33e07 to 6a056f0 Compare October 18, 2022 08:47
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 00s
✔️ build-ansible-collection SUCCESS in 4m 53s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 24s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 41s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 01s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 5m 59s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 54s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 06s
✔️ cloud-tox-py3 SUCCESS in 3m 32s
✔️ ansible-test-splitter SUCCESS in 3m 03s
✔️ integration-amazon.aws-1 SUCCESS in 34m 07s
✔️ integration-amazon.aws-2 SUCCESS in 48m 48s
✔️ integration-amazon.aws-3 SUCCESS in 24m 42s
✔️ integration-amazon.aws-4 SUCCESS in 55m 41s
✔️ integration-amazon.aws-5 SUCCESS in 41m 15s
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 2m 18s

@tremble tremble marked this pull request as ready for review October 18, 2022 11:25
@tremble tremble added mergeit Merge the PR (SoftwareFactory) backport-5 PR should be backported to the stable-5 branch labels Oct 18, 2022
@softwarefactory-project-zuul

This comment was marked as outdated.

@tremble
Copy link
Contributor Author

tremble commented Oct 18, 2022

recheck

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 11s
✔️ build-ansible-collection SUCCESS in 5m 01s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 53s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 47s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 8m 58s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 31s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 19s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 13s
✔️ cloud-tox-py3 SUCCESS in 3m 15s
✔️ ansible-test-splitter SUCCESS in 3m 10s
✔️ integration-amazon.aws-1 SUCCESS in 55m 36s
✔️ integration-amazon.aws-2 SUCCESS in 46m 14s
✔️ integration-amazon.aws-3 SUCCESS in 26m 26s
✔️ integration-amazon.aws-4 SUCCESS in 47m 57s
✔️ integration-amazon.aws-5 SUCCESS in 39m 40s
⚠️ integration-amazon.aws-6 SKIPPED
⚠️ integration-amazon.aws-7 SKIPPED
⚠️ integration-amazon.aws-8 SKIPPED
⚠️ integration-amazon.aws-9 SKIPPED
⚠️ integration-amazon.aws-10 SKIPPED
⚠️ integration-amazon.aws-11 SKIPPED
⚠️ integration-amazon.aws-12 SKIPPED
⚠️ integration-amazon.aws-13 SKIPPED
⚠️ integration-amazon.aws-14 SKIPPED
⚠️ integration-amazon.aws-15 SKIPPED
⚠️ integration-amazon.aws-16 SKIPPED
⚠️ integration-amazon.aws-17 SKIPPED
⚠️ integration-amazon.aws-18 SKIPPED
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
✔️ ansible-test-changelog SUCCESS in 2m 18s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit e3f091e into ansible-collections:main Oct 18, 2022
@patchback
Copy link

patchback bot commented Oct 18, 2022

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

❌ Failed to cleanly apply e3f091e on top of patchback/backports/stable-5/e3f091e620c96c0dddd12fda16fb0dae4389ee19/pr-1181

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

@tremble tremble deleted the sanity/2022-10-18 branch October 21, 2022 07:32
tremble added a commit to tremble/amazon.aws that referenced this pull request Oct 28, 2022
Partial backport of ansible-collections#1181

##### SUMMARY

Minor linting cleanup

missing whitespace

##### ISSUE TYPE

Feature Pull Request

##### COMPONENT NAME
plugins/inventory/aws_ec2.py
plugins/modules/s3_object_info.py

##### ADDITIONAL INFORMATION

Original Reviews:
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Oct 28, 2022
Backport Linting cleanup from 1181

Partial backport of #1181
SUMMARY
Minor linting cleanup

missing whitespace

ISSUE TYPE
Feature Pull Request
COMPONENT NAME
plugins/inventory/aws_ec2.py
plugins/modules/s3_object_info.py
ADDITIONAL INFORMATION
Original Reviews:
Reviewed-by: Alina Buzachis
Reviewed-by: Gonéri Le Bouder

Reviewed-by: Alina Buzachis <None>
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Oct 28, 2022
Backport Linting cleanup from 1181

Partial backport of ansible-collections#1181
SUMMARY
Minor linting cleanup

missing whitespace

ISSUE TYPE
Feature Pull Request
COMPONENT NAME
plugins/inventory/aws_ec2.py
plugins/modules/s3_object_info.py
ADDITIONAL INFORMATION
Original Reviews:
Reviewed-by: Alina Buzachis
Reviewed-by: Gonéri Le Bouder

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
ecs_service -- Capacity provider strategy

SUMMARY
Fixes ansible-collections#1137
Per request, allow for the user to provide a capacity_provider_strategy when creating or updating an ECS service. This capacity_provider_strategy is a list of 1-6 dictionaries. The new capacity_provider_strategy is mutually exclusive with launch_type and an existing service cannot be changed from one to the other.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION
The new parameter is optional and non-default. If neither launch_type or capacity_provider_strategy are provided, the new service will default to EC2 launch_type. The module handles the mutually exclusivity and also catches and fails cleanly when trying to change an existing service from launch_type to capacity_provider_strategy or vice versa.
Tested pretty thoroughly against ansible 2.9.27. Updated parameters, examples, and return objects provided.
Before merge the module will just ignore the capacity_provider_strategy and default to EC2 launch_type.
After merge the module will handle either launch_type or capacity_provider_strategy and create/update the service as necessary.
- community.aws.ecs_service:
    state: present
    name: test-service
    cluster: test-cluster
    task_definition: test-task-definition
    desired_count: 1
    capacity_provider_strategy:
      - capacity_provider: test-capacity-provider-1
        weight: 1
        base: 0

Reviewed-by: Markus Bergholz <git@osuv.de>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
ecs_service -- Capacity provider strategy

SUMMARY
Fixes ansible-collections#1137
Per request, allow for the user to provide a capacity_provider_strategy when creating or updating an ECS service. This capacity_provider_strategy is a list of 1-6 dictionaries. The new capacity_provider_strategy is mutually exclusive with launch_type and an existing service cannot be changed from one to the other.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION
The new parameter is optional and non-default. If neither launch_type or capacity_provider_strategy are provided, the new service will default to EC2 launch_type. The module handles the mutually exclusivity and also catches and fails cleanly when trying to change an existing service from launch_type to capacity_provider_strategy or vice versa.
Tested pretty thoroughly against ansible 2.9.27. Updated parameters, examples, and return objects provided.
Before merge the module will just ignore the capacity_provider_strategy and default to EC2 launch_type.
After merge the module will handle either launch_type or capacity_provider_strategy and create/update the service as necessary.
- community.aws.ecs_service:
    state: present
    name: test-service
    cluster: test-cluster
    task_definition: test-task-definition
    desired_count: 1
    capacity_provider_strategy:
      - capacity_provider: test-capacity-provider-1
        weight: 1
        base: 0

Reviewed-by: Markus Bergholz <git@osuv.de>
Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
ecs_service -- Capacity provider strategy

SUMMARY
Fixes ansible-collections#1137
Per request, allow for the user to provide a capacity_provider_strategy when creating or updating an ECS service. This capacity_provider_strategy is a list of 1-6 dictionaries. The new capacity_provider_strategy is mutually exclusive with launch_type and an existing service cannot be changed from one to the other.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ecs_service
ADDITIONAL INFORMATION
The new parameter is optional and non-default. If neither launch_type or capacity_provider_strategy are provided, the new service will default to EC2 launch_type. The module handles the mutually exclusivity and also catches and fails cleanly when trying to change an existing service from launch_type to capacity_provider_strategy or vice versa.
Tested pretty thoroughly against ansible 2.9.27. Updated parameters, examples, and return objects provided.
Before merge the module will just ignore the capacity_provider_strategy and default to EC2 launch_type.
After merge the module will handle either launch_type or capacity_provider_strategy and create/update the service as necessary.
- community.aws.ecs_service:
    state: present
    name: test-service
    cluster: test-cluster
    task_definition: test-task-definition
    desired_count: 1
    capacity_provider_strategy:
      - capacity_provider: test-capacity-provider-1
        weight: 1
        base: 0

Reviewed-by: Markus Bergholz <git@osuv.de>
Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-5 PR should be backported to the stable-5 branch community_review feature This issue/PR relates to a feature request inventory inventory plugin mergeit Merge the PR (SoftwareFactory) module_utils module_utils module module needs_maintainer needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants