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.

Sign up for GitHub

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

Jump to bottom

Initial support for AWS Batch Job Definitions #26700

Closed
wants to merge 7 commits into from
Closed

Initial support for AWS Batch Job Definitions #26700

wants to merge 7 commits into from

Conversation

jonmer85
Copy link
Contributor

SUMMARY

This adds automation support for the following AWS Batch Job Definitions
ISSUE TYPE

Feature Pull Request
COMPONENT NAME

amazon module

ANSIBLE VERSION

ansible 2.2.2.0
config file =
configured module search path = Default w/o overrides

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 feature_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 12, 2017
@jonmer85 jonmer85 mentioned this pull request Jul 12, 2017
Closed
@jctanner jctanner mentioned this pull request Jul 12, 2017
Closed
@bcoca bcoca added aws new_module This PR includes a new module. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_triage Needs a first human triage before being processed. labels Jul 12, 2017
@calfonso calfonso added cloud support:community This issue/PR relates to code supported by the Ansible community. labels Jul 12, 2017
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed new_module This PR includes a new module. labels Jul 12, 2017
@ansibot
Copy link
Contributor

ansibot commented Jul 12, 2017

cc @willthames
click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jul 20, 2017

@Constantin07 @Etherdaemon @Lujeni @MichaelBaydoun @Sodki @adq @akazakov @alachaum @amir343 @bekelchik @bpennypacker @brandond @carsongee @dkhenry @fiunchinho @garethr @gunzy83 @j-carl @jarv @Java1Guy @jmenga @joelthompson @jonhadfield @jsdalton @jsmartin @kaczynskid @leedm777 @linuxdynasty @loia @lwade @MichaelBaydoun @michaeljs1990 @minichate @mjschultz @mmochan @nadirollo @nand0p @naslanidis @NickBall @pjodouin @psykotox @pwnall @RickMendes @roadmapper @ryansydnor @scicoin-project @scottanderson42 @seiffert @shepdelacreme @silviud @simplesteph @steynovich @tastychutney @tedder @tgerla @timmahoney @tombamford @whiter @wilvk @wimnat @zacblazic @zbal @zeekin @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_module This PR includes a new module. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 20, 2017
ryansb
ryansb suggested changes Aug 3, 2017
View reviewed changes
lib/ansible/modules/cloud/amazon/batch_job_definition.py Outdated

DOCUMENTATION = '''
---
module: batch_job_definition
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this one: since there's a flat namespace, please prefix this as aws_batch_job_definition.

Copy link

Choose a reason for hiding this comment

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

fixed

lib/ansible/modules/cloud/amazon/batch_job_definition.py Outdated
# logger.setLevel(logging.DEBUG)


class AWSConnection:
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is only ever used once, I think it would be easier to read to just create the different client/resource connections and pass those in.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 3, 2017
@ansibot ansibot removed the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Aug 16, 2017
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 15, 2017
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Sep 25, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 26, 2018
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 26, 2018
@ansibot
Copy link
Contributor

ansibot commented Jan 26, 2018

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

lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E317 "image" is marked as required but specifies a default. Arguments with a default should not be marked as required
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E317 "job_definition_name" is marked as required but specifies a default. Arguments with a default should not be marked as required
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E317 "memory" is marked as required but specifies a default. Arguments with a default should not be marked as required
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E317 "type" is marked as required but specifies a default. Arguments with a default should not be marked as required
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E317 "vcpus" is marked as required but specifies a default. Arguments with a default should not be marked as required
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E322 "job_definition_arn" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E323 "aws_region" is listed in DOCUMENTATION.options, but not accepted by the module
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E323 "ec2_region" is listed in DOCUMENTATION.options, but not accepted by the module

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Jan 26, 2018
@ansibot
Copy link
Contributor

ansibot commented Jan 26, 2018

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

lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E322 "job_definition_arn" is listed in the argument_spec, but not documented in the module
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E323 "aws_region" is listed in DOCUMENTATION.options, but not accepted by the module
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py:0:0: E323 "ec2_region" is listed in DOCUMENTATION.options, but not accepted by the module

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Jan 26, 2018
ryansb
ryansb reviewed Jan 29, 2018
View reviewed changes
lib/ansible/modules/cloud/amazon/aws_batch_job_definition.py
@@ -0,0 +1,526 @@
#!/usr/bin/python
# Copyright (c) 2017 Jon Meran <jonathan.meran@sonos.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should follow copyright guideline http://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#copyright and have author info in module author field.

jonmeransonos and others added 6 commits January 30, 2018 13:46
@jonmeransonos @s-hertel
Initial support for AWS Batch Job Definitions
58ed7f3
@supreethSonos @s-hertel
Changes include:
dd8bd26 
	1. Renamed file from batch_job_definition.py to aws_batch_job_definition.py
	2. Fixed ANSIBLE_METADATA version
	3. Updated the RETURN documentation
	4. Used Try Except ImportError block while importing non standard python libraries
	5. Updated the remove_job_definition() method to use deregister_job_definition()
	6. Updated set_api_params() method
	7. Removed the response getting embedded in ansible_facts.
	8. Updated main() to convert the final result to snake_dict using camel_dict_to_snake_dict() to maintain consistency.
	9. Added region and job_definition_arn in argument_spec
@supreethSonos @s-hertel
Fixed Botocore error
131b71e
@supreethSonos @s-hertel
Revert "Fixed Botocore error"
7ec6cfe 
This reverts commit 0db6ddef5ba00d4cd063eb3f13243189c0ae9ad2.
@s-hertel
Update copyright
b4c1206 
Add traceback to exceptions

Remove exception handling around the connection since boto3_conn handles it

Add ec2 documentation fragment

Region is not required because it may be provided as an environment variable

Remove default=None from the arg spec since it is the default and cannot be used with required=true

Add aliases for region

Document job_definition_arn
@s-hertel
Import shared code
0ee7eb8
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed new_contributor This PR is the first contribution by a new community member. labels Jan 30, 2018
@ansibot
Copy link
Contributor

ansibot commented Jan 30, 2018

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

lib/ansible/module_utils/aws/batch.py:98:11: undefined-variable Undefined variable 'snake_dict_to_camel_dict'

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 30, 2018
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. and removed ci_verified Changes made in this PR are causing tests to fail. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. labels Jan 30, 2018
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jan 30, 2018
@ryansb
Copy link
Contributor

ryansb commented Jan 30, 2018

Merged as 05f765b

@ryansb ryansb closed this Jan 30, 2018
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 5, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@supreethSonos supreethSonos supreethSonos left review comments

@ryansb ryansb ryansb approved these changes

@s-hertel s-hertel s-hertel approved these changes

Assignees
No one assigned
Labels
affects_2.4 This issue/PR affects Ansible v2.4 aws cloud feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_module This PR includes a new module. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@jonmer85 @ansibot @ryansb @s-hertel @supreethSonos @calfonso @bcoca @jonmeransonos