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 module: Cloud Volumes for AWS, active Directory #61342

Merged
merged 3 commits into from Aug 29, 2019

Conversation

@carchi8py
Copy link
Contributor

commented Aug 26, 2019

SUMMARY

new module: Cloud Volumes for AWS, active directory

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME
  • aws_netapp_cvs_active_directory.py
ADDITIONAL INFORMATION

@carchi8py carchi8py changed the title new module: Cloud Volumes for AWS, filesystem new module: Cloud Volumes for AWS, active Directory Aug 26, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

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

@carchi8py carchi8py force-pushed the carchi8py:1834 branch from ed78d78 to ea7b652 Aug 27, 2019

@ansibot ansibot removed the needs_rebase label Aug 27, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_active_directory.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test metaclass-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_active_directory.py:0:0: missing: __metaclass__ = type

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

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_active_directory.py:0:0: E338 Argument 'state' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_cluster_ha.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_firmware_upgrade.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_info.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_motd.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_ndmp.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_object_store.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_ports.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_qos_adaptive_policy_group.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_qtree.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_quotas.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_volume_autosize.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_vscan.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/na_ontap_vserver_cifs_security.py:0:0: E337 Argument 'use_rest' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_password' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_url' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'api_username' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_facts.py:0:0: E337 Argument 'ssid' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_password' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_url' in argument_spec uses default type ('str') but documentation doesn't define type
lib/ansible/modules/storage/netapp/netapp_e_volume_copy.py:0:0: E338 Argument 'api_username' in argument_spec uses default type ('str') but documentation doesn't define type

click here for bot help

@lonico
lonico approved these changes Aug 28, 2019
Copy link
Contributor

left a comment

shipit


def get_activedirectory(self, activeDirectoryId=None):
if activeDirectoryId is None:
return None

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

The else: is not really necessary here since there is a return just above.

But it does not change the behavior.

self.na_helper.changed = True
if 'domain' in self.parameters and self.parameters['domain'] is not None:
ad_exists = self.get_activedirectory(updated_active_directory['domain'])
if ad_exists:

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

I would report an error here, as we did not complete the action.

But this change can be delayed.

ad_module()
print('Info: %s' % exc.value.args[0]['msg'])

def test_module_fail_when_required_args_present(self):

This comment has been minimized.

Copy link
@lonico

lonico Aug 28, 2019

Contributor

?
I'm not sure I understand what this is supposed to do.

Can be delayed.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

The test ansible-test sanity --test future-import-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_active_directory.py:0:0: missing: from __future__ import (absolute_import, division, print_function)

The test ansible-test sanity --test metaclass-boilerplate [explain] failed with 1 error:

test/units/modules/cloud/amazon/test_aws_netapp_cvs_active_directory.py:0:0: missing: __metaclass__ = type

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_netapp_cvs_active_directory.py:0:0: E338 Argument 'state' in argument_spec uses default type ('str') but documentation doesn't define type

click here for bot help

@ansibot ansibot added the ci_verified label Aug 28, 2019

@ansibot ansibot removed the ci_verified label Aug 28, 2019

@lonico
lonico approved these changes Aug 28, 2019
Copy link
Contributor

left a comment

shipit

@ansibot ansibot added core_review and removed needs_revision labels Aug 28, 2019

@thedoubl3j
Copy link
Contributor

left a comment

CI is good, tests are added and docs look clean. lgtm. shipit

@ansibot ansibot added shipit and removed core_review labels Aug 29, 2019

@thedoubl3j thedoubl3j merged commit 53b2a26 into ansible:devel Aug 29, 2019

1 check passed

Shippable Run 140618 status is SUCCESS.
Details

@sivel sivel removed the needs_triage label Aug 29, 2019

adharshsrivatsr added a commit to adharshsrivatsr/ansible that referenced this pull request Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.