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

new replication subnet group module #56075

Open
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@ruimoreira
Copy link
Contributor

commented May 4, 2019

SUMMARY

New module for replication subnet group

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

dms_replication_subnet_group

ADDITIONAL INFORMATION
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

@ruimoreira, 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

ruimoreira added some commits May 6, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

@BobBoldin @Constantin007 @Constantin07 @Deepakkothandan @Etherdaemon @Java1Guy @Madhura-CSI @MichaelBaydoun @Sodki @Zeekin @adq @akazakov @alachaum @amir343 @anryko @bekelchik @brandond @captainkerk @chenl87 @defunctio @dennisconrad @dkhenry @fiunchinho @fivethreeo @flowerysong @garethr @gobins @gunzy83 @gurumaia @hsingh @hyperized @iiibrad @infectsoldier @j-carl @jarv @jimbydamonk @jmenga @joelthompson @jonhadfield @jonmer85 @joshsouza @jsdalton @jsmartin @kaczynskid @leedm777 @linuxdynasty @loia @lwade @michaeljs1990 @minichate @mjschultz @mmochan @nand0p @naslanidis @nathanwebsterdotme @nerzhul @nickball @orthanc @ozbillwang @piontas @pjodouin @prasadkatti @psykotox @ptux @pwnall @raags @rafaeldriutti @rickmendes @roadmapper @rrey @ryansydnor @scicoin-project @scottanderson42 @sdubrul @shepdelacreme @silviud @slapula @steynovich @tastychutney @tgerla @timmahoney @tomislacker @tsiganenok @viper233 @whiter @willricardo @wilvk @wimnat @yaakov-github @zacblazic @zbal @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

ruimoreira added some commits May 6, 2019

changed tests to remove static subnet ids, alaso added code to create…
… , VPC, and subnets and destroy them in the end
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/dms_replication_subnet_group.py
# This file is part of Ansible
#
# Ansible is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by

This comment has been minimized.

Copy link
@resmo

resmo May 17, 2019

Member

please use short notation of the GPL header

This comment has been minimized.

Copy link
@ruimoreira

ruimoreira May 21, 2019

Author Contributor

same

global module
module = AnsibleAWSModule(
argument_spec=argument_spec,
required_if=[],

This comment has been minimized.

Copy link
@resmo

resmo May 17, 2019

Member

can be removed

This comment has been minimized.

Copy link
@ruimoreira

ruimoreira May 21, 2019

Author Contributor

same here

exit_message = None
changed = False
if not HAS_BOTO3:
module.fail_json(msg='boto3 required for this module')

This comment has been minimized.

Copy link
@resmo

resmo May 17, 2019

Member

there is a helper "missing dependency" error output. please see exsiting modules

This comment has been minimized.

Copy link
@ruimoreira

ruimoreira May 21, 2019

Author Contributor

can you please point me in the right direction here ?

if replication_subnet_exists(subnet_group):
if compare_params(subnet_group["ReplicationSubnetGroups"][0]):
changed = True
exit_message = modify_replication_subnet_group(dmsclient)

This comment has been minimized.

Copy link
@resmo

resmo May 17, 2019

Member

why not implement check mode?

if not module.check_mode:
    exit_message = modify_replication_subnet_group(dmsclient)
else: 
    exit_message = " a phrase that makes sense in check mode"
def main():
argument_spec = dict(
state=dict(choices=['present', 'absent'], default='present'),
subnetgroupidentifier=dict(required=True),

This comment has been minimized.

Copy link
@resmo

resmo May 17, 2019

Member

not a blocker but I find these long params very hard to read and because the module is named dms_replication_subnet_group, why not just skip subnetgroup and only use identifier and description ?

This comment has been minimized.

Copy link
@ruimoreira

ruimoreira May 21, 2019

Author Contributor

@resmo this has been addressed

- shorter parameter names
- removed unused dict for requried_if
- shorter GPL Header
Rui Moreira Rui Moreira

@ansibot ansibot removed the needs_revision label May 21, 2019

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.