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

Sanity: Fix argument_spec and documentation #133

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

Akasurde
Copy link
Member

@Akasurde Akasurde commented Jul 9, 2020

SUMMARY

Signed-off-by: Abhijeet Kasurde [email protected]

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

plugins/modules/cloudformation_stack_set.py
plugins/modules/cloudfront_distribution.py
plugins/modules/cloudfront_invalidation.py
plugins/modules/cloudwatchevent_rule.py
plugins/modules/data_pipeline.py
plugins/modules/dynamodb_table.py
plugins/modules/ec2_asg.py
plugins/modules/ec2_customer_gateway_info.py
plugins/modules/ec2_elb.py
plugins/modules/ec2_elb_info.py
plugins/modules/ec2_instance.py
plugins/modules/ec2_instance_info.py
plugins/modules/ec2_launch_template.py
plugins/modules/ec2_lc.py
plugins/modules/ec2_lc_info.py
plugins/modules/ec2_metric_alarm.py
plugins/modules/ec2_placement_group_info.py
plugins/modules/ec2_transit_gateway_info.py
plugins/modules/ec2_vpc_endpoint.py
plugins/modules/ec2_vpc_endpoint_info.py
plugins/modules/ec2_vpc_igw_info.py
plugins/modules/ec2_vpc_nacl.py
plugins/modules/ec2_vpc_nacl_info.py
plugins/modules/ec2_vpc_nat_gateway_info.py
plugins/modules/ec2_vpc_peering_info.py
plugins/modules/ec2_vpc_route_table.py
plugins/modules/ec2_vpc_vgw_info.py
plugins/modules/ec2_vpc_vpn.py
plugins/modules/ec2_vpc_vpn_info.py
plugins/modules/ecs_attribute.py
plugins/modules/ecs_service.py
plugins/modules/ecs_service_info.py
plugins/modules/ecs_task.py
plugins/modules/ecs_taskdefinition.py
plugins/modules/efs.py
plugins/modules/efs_info.py
plugins/modules/elasticache.py
plugins/modules/elasticache_subnet_group.py
plugins/modules/elb_application_lb.py
plugins/modules/elb_application_lb_info.py
plugins/modules/elb_classic_lb.py
plugins/modules/elb_classic_lb_info.py
plugins/modules/elb_instance.py
plugins/modules/elb_network_lb.py
plugins/modules/elb_target_group.py
plugins/modules/elb_target_group_info.py
plugins/modules/iam.py
plugins/modules/iam_group.py
plugins/modules/iam_role.py
plugins/modules/iam_user.py
plugins/modules/lambda.py
plugins/modules/rds.py
plugins/modules/rds_instance.py
plugins/modules/rds_subnet_group.py
plugins/modules/redshift.py
plugins/modules/redshift_subnet_group.py
plugins/modules/route53.py
plugins/modules/route53_info.py
plugins/modules/s3_bucket_notification.py
plugins/modules/s3_lifecycle.py
plugins/modules/sns_topic.py
tests/sanity/ignore-2.11.txt

@Akasurde Akasurde force-pushed the sanity_fix_iii branch 3 times, most recently from 71b666d to ea61cf0 Compare July 9, 2020 06:26
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these, I'm very glad to see the sanity ignore list dropping back down again.

Nit: Part of me feels like calling the elements/options fixes "doc" fixes in the commit is wrong.

I've double checked the ecs_service options and any alternative settings would have resulted in errors from the API, so we should be good there.

The test failures appear to be due to API ratelimiting/throttling. It's worth triggering a retry later, but with the significant increase in versions of Ansible we're (concurrently) testing against we may be about to see a significant increase in failures when we try to test multiple groups at the same time.

@Akasurde
Copy link
Member Author

Akasurde commented Jul 9, 2020

@tremble Thanks for reviewing. I will amend the commit message when I will retry this again.

@Akasurde Akasurde changed the title Docs: sanity fixes Sanity: Fix argument_spec and documentation Jul 9, 2020
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Akasurde

Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One change actually, noted inline. I'm not certain that's what's causing tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/iam_instance_role.yml to fail so there may be something else still. I'd recommend launching the ec2_instance tests locally once before pushing to github so you don't have to fight so much with API rate limits.

plugins/modules/ec2_instance.py Outdated Show resolved Hide resolved
Signed-off-by: Abhijeet Kasurde <[email protected]>
@jillr
Copy link
Collaborator

jillr commented Jul 15, 2020

The ec2_instance tests pass reliably locally now, the CI flakes were just api thresholds and timeouts from how many tests this one ran in parallel. We have all green now after rerunning a few failures and I'm happy with this one. Thanks again.

@jillr jillr merged commit 059cf9e into ansible-collections:main Jul 15, 2020
@Akasurde Akasurde deleted the sanity_fix_iii branch July 16, 2020 02:34
@@ -321,11 +321,11 @@ def main():
family=dict(required=False, type='str'),
revision=dict(required=False, type='int'),
force_create=dict(required=False, default=False, type='bool'),
containers=dict(required=False, type='list'),
containers=dict(required=False, type='list', elements='str'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jillr I think its dict instead of str

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #238

alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 14, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 15, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 16, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 17, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants