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

Added gandi_livedns module #56952

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@gthiemonge
Copy link

commented May 25, 2019

SUMMARY

Ansible module to control Gandi DNS service (https://www.gandi.net/en/domain)

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME
  • gandi_livedns
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 25, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 25, 2019

@gthiemonge this PR contains more than one new module.

Please submit only one new module per pull request. For a detailed explanation, please read the grouped modules documentation

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 25, 2019

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

lib/ansible/modules/net_tools/gandi_livedns.py:0:0: E307 version_added should be '2.9'. Currently '1.0'
lib/ansible/modules/net_tools/gandi_livedns_facts.py:0:0: E307 version_added should be '2.9'. Currently '1.0'

click here for bot help

@ansibot ansibot added the ci_verified label May 25, 2019

@gthiemonge gthiemonge force-pushed the gthiemonge:gandi_livedns branch from 8c3affc to f698d3d May 25, 2019

@gthiemonge gthiemonge changed the title Added gandi_livedns and gandi_livedns_facts modules Added gandi_livedns module May 25, 2019

@bcoca bcoca removed the needs_triage label May 28, 2019

@ansibot ansibot added the stale_ci label Jun 5, 2019

@resmo resmo self-requested a review Jun 7, 2019

@resmo
Copy link
Member

left a comment

Well done, I only found an issue about licensing. But I have some questions, you put almost all logic into module_utils, usually module specific code not shared across other modules should go into the one module directly. Are there plans to use this in other gandi modules?

Show resolved Hide resolved lib/ansible/module_utils/gandi_livedns_api.py Outdated
if not module.params['zone'] and not module.params['domain']:
module.fail_json(msg="At least one of zone and domain parameters need to be defined.")
if module.params['state'] == 'present' and module.params['ttl'] is None:
module.params['ttl'] = 10800

This comment has been minimized.

Copy link
@resmo

resmo Jun 8, 2019

Member

is there a reason why you don't default this in the arg_spec?

This comment has been minimized.

Copy link
@gthiemonge

gthiemonge Jun 18, 2019

Author
  1. at least one of zone or domain is required, how could I define this in arg_spec?

  2. ttl can be used as a filter when state is absent, and may be None if the user doesn't want to pass it explicitly

  • {name: foo, type: A, state: absent} will remove foo A entry regardless the value of ttl
  • {name: foo, type: A, ttl: 3600, state: absent} will remove foo A entry only if its ttl is 3600

So ttl is defaulted to something (10800) only if state is present.
Is there a way to have a default value in arg_spec only when state is present?

@ansibot ansibot added needs_revision and removed core_review labels Jun 8, 2019

Gregory Thiemonge

@gthiemonge gthiemonge force-pushed the gthiemonge:gandi_livedns branch from f698d3d to f5066e2 Jun 18, 2019

@ansibot ansibot removed the stale_ci label Jun 18, 2019

@gthiemonge

This comment has been minimized.

Copy link
Author

commented Jun 18, 2019

Are there plans to use this in other gandi modules?

I plan to send a PR for a gandi_livedns_facts module that makes use of this api.

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.