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

Add info module for Azure batch account #57816

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
7 participants
@JunyiYi
Copy link
Contributor

commented Jun 13, 2019

SUMMARY

This PR introduces the info module (previously called facts module) of Azure batch account.

COMPONENT NAME

azure_rm_batchaccount
azure_rm_batchaccount_info

ADDITIONAL INFORMATION

This code is auto generated. Please do not manually change the content.

For code changes, please also contact @zikalino as well.

JunyiYi added some commits Jun 13, 2019

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

@JunyiYi, just so you are aware we have a dedicated Working Group for azure.
You can find other people interested in this in #ansible-azure on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

@Madhura-CSI @brusMX @caohai @cperrin88 @devigned @gustavomcarmo @haroldwongms @iphilpot @jeking3 @julienstroheker @lmazuel @needgithubid @obsoleted @sozercan @techknowlogick @trstringer @yaweiw @yuwzho

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

@MyronFanQiu

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

@zikalino @yungezz Hello. Please review this PR when you are free.

Show resolved Hide resolved lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py
options:
resource_group:
description:
- The name of the resource group in which to create the Batch Account.

This comment has been minimized.

Copy link
@yungezz

yungezz Jun 17, 2019

Contributor

indention

This comment has been minimized.

Copy link
@JunyiYi

JunyiYi Jun 17, 2019

Author Contributor

Is it possible to use this type of indentation since this indentation is also valid in yaml? Actually I do not have control over this indentation, it is restricted by the Ruby library and Google's code also uses this type of indentation.

This comment has been minimized.

Copy link
@yungezz

yungezz Jun 17, 2019

Contributor

This is not consistent with current Ansible indention style. let's see community feedback.

contains:
id:
description:
- The ID of the Batch account.

This comment has been minimized.

Copy link
@yungezz

yungezz Jun 17, 2019

Contributor

indention

Show resolved Hide resolved lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py Outdated
Show resolved Hide resolved lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py Outdated
def exec_module(self, **kwargs):
for key in self.module_arg_spec:
setattr(self, key, kwargs[key])
self.mgmt_client = self.get_mgmt_svc_client(BatchManagementClient,

This comment has been minimized.

Copy link
@yungezz

yungezz Jun 17, 2019

Contributor

api version?

This comment has been minimized.

Copy link
@JunyiYi

JunyiYi Jun 17, 2019

Author Contributor

@zikalino Do we need api_version here? I remembered in your guideline we do not need to include it.


if (self.name):
self.results['items'] = self.get()
else:

This comment has been minimized.

Copy link
@yungezz

yungezz Jun 17, 2019

Contributor

also need to support list batch account in this subscription, when resource_group is None

This comment has been minimized.

Copy link
@zikalino

zikalino Jun 17, 2019

Contributor

there's an issue for that: Azure/magic-module-specs#7

This comment has been minimized.

Copy link
@JunyiYi

JunyiYi Jun 17, 2019

Author Contributor

@zikalino @yungezz Could any one provide the actual code in this PR? So I can adjust the tool to match exactly the same as it.

@yungezz
Copy link
Contributor

left a comment

comments inline

@yungezz

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

I think test will be added also, right? and pls add this file to alias file of azure_rm_batchaccount test.

@JunyiYi

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

I think test will be added also, right? and pls add this file to alias file of azure_rm_batchaccount test.

@zikalino would you please add the test to thie PR? And please also add the feature of "listing batch account in this subscription" as well. I will adjust the tooling accordingly.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

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

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:104:161: E501 line too long (165 > 160 characters)

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

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'name' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'password' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'resource_group' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'tags' in argument_spec defines type as 'list' but documentation doesn't define type

click here for bot help

@ansibot ansibot added the ci_verified label Jun 17, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

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

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'name' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'password' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'resource_group' in argument_spec defines type as 'str' but documentation doesn't define type
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'tags' in argument_spec defines type as 'list' but documentation doesn't define type

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:170:11: E111 indentation is not a multiple of four
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:172:11: E111 indentation is not a multiple of four
lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:174:11: E111 indentation is not a multiple of four

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

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'password' in argument_spec defines type as 'str' but documentation doesn't define type

click here for bot help

@ansibot ansibot removed the ci_verified label Jun 18, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

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

lib/ansible/modules/cloud/azure/azure_rm_batchaccount_info.py:0:0: E337 Argument 'password' in argument_spec defines type as 'str' but documentation doesn't define type

click here for bot help

@ansibot ansibot added the stale_ci label Jun 27, 2019

@Fred-sun

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

@JunyiYi Thanks for your contribution! Please finished the PR changed when you're free. I will invite reviewers to review.

@JunyiYi

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

Hi @Fred-sun , I think I have already resolved all issues. The one presented by the CI job is unclear to me, because I do not even have any passwords in my code base. Can you take a look at it? Thanks.

@Fred-sun

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2019

@MyronFanQiu Please help take a look this issue when you're free? CI verify failed because of 'password', but this module does not define 'password'! Thank you very much!

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.