[nxos_interfaces] resource module POC by trishnaguha · Pull Request #2 · ansible/network

trishnaguha · 2019-03-11T10:43:10Z

Signed-off-by: Trishna Guha trishnaguha17@gmail.com

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

privateip · 2019-03-19T13:29:27Z

+from ansible.module_utils.six import iteritems
+
+
+class ArgspecBase(object):


If you are planning to provide direct attribute access to dict values, you should implement __delattr__ as well.

How will other module arguments be implemented? required_if, required_together, etc

@privateip Thanks for the review. So Base class code is actually not in use anymore, it's the old code. I have updated the Base class to be empty now. This is actually going to be achieved by the code generator.

Other module arguments such as required_if would be used the following way:

diff --git a/library/nxos_interfaces.py b/library/nxos_interfaces.py index a636aa6..e902e50 100644 --- a/library/nxos_interfaces.py +++ b/library/nxos_interfaces.py @@ -50,6 +50,7 @@ def main(): """ main entry point for module execution """ module = AnsibleModule(argument_spec=Interface.argument_spec, + required_if=Interface.required_if, supports_check_mode=True) result = Interface(module).execute_module() diff --git a/module_utils/argspec/nxos/interfaces/interfaces.py b/module_utils/argspec/nxos/interfaces/interfaces.py index a97550e..62cefd6 100644 --- a/module_utils/argspec/nxos/interfaces/interfaces.py +++ b/module_utils/argspec/nxos/interfaces/interfaces.py @@ -19,3 +19,5 @@ class InterfaceArgs(ArgspecBase): 'state': dict(default='merged', choices=['merged', 'replaced', 'overriden', 'deleted']), 'config': dict(type='list', elements='dict', options=config_spec) } + + required_if = [('state', 'merged', ('mode',))]

privateip · 2019-03-19T13:30:57Z

+            if key in self.argument_spec:
+                setattr(self, key, value)
+
+    def __getattr__(self, key):


This method should raise an AttributeError if the request key is not in the dict. Also, in otder to avoid recursive lookups, the key should be checked against __dict__ not self.values.

Attributes that begin with _ should only be checked against the internal structure not the instance variable, imho

privateip · 2019-03-19T13:31:29Z

+    argument_spec = {}
+
+    def __init__(self, **kwargs):
+        self.values = {}


The instance variable should be named self._values imho to denote its an internal implementation

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

ganeshrn · 2019-03-25T13:04:59Z

+class FactsArgs(ArgspecBase):
+
+    argument_spec = {
+        'gather_subset': dict(default=['all'], type='list')


Can we add individual supported keys, so that it is clear to the user which all option values it can support?

ganeshrn · 2019-03-25T13:05:45Z

+        self._module = module
+        self._connection = self._get_connection()
+
+    def _get_connection(self):


This API can be moved to the base class?

@ganeshrn which base class are you referring to?

ganeshrn · 2019-03-25T13:10:37Z

+class Facts(FactsArgs, FactsBase):
+
+    VALID_SUBSETS = [
+        'net_configuration_interfaces',


Add this value in the argspec for class FactsArgs(ArgspecBase)

'gather_subset': dict(default=['all'], choices=['net_configuration_interfaces', 'all'], type='list')

ganeshrn · 2019-03-25T13:20:37Z

@@ -0,0 +1,4 @@
+class ArgspecBase(object):


What is the purpose of this class?

This is empty as of now. When we merge this POC with code generator that will generate the argspec, we can decide then?

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

NilashishC · 2019-04-01T09:38:41Z

+        :rtype: A text string
+        :returns: A text string if match is found
+        """
+        match = re.search(r'%s (.+)(\n|$)' % arg, cfg, re.M)


Since this is a base class for Facts, can we make this more generic by making this accept the pattern instead of hardcoding it? For e.g., something like this which is being utilized here.

@NilashishC this is base class only for NXOS facts. This method is not a generic method for all kind of regex matches, rather for strings that start with the key we are looking for the value right after it.
For example,
description Configured by Ansible
feature nxapi
hostname test

The example you shared has the exact same regex pattern for all the keys, so the regex pattern can act as a generic pattern in the method definition for all those keys IMO.
Maybe we need a better method name?

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

pabelanger · 2019-04-25T00:11:35Z

recheck

ansible-zuul · 2019-04-25T00:16:21Z

Build failed.

noop : SUCCESS in 0s
ansible-tox-linters : FAILURE in 3m 31s

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

ansible-zuul · 2019-04-30T11:07:17Z

Build failed.

ansible-tox-linters : FAILURE in 3m 32s

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

ansible-zuul · 2019-05-02T13:55:30Z

Build failed.

ansible-tox-linters : FAILURE in 4m 30s

ansible-zuul · 2019-05-20T15:18:54Z

Build failed.

ansible-tox-linters : FAILURE in 5m 40s

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

ansible-zuul · 2019-05-23T10:28:59Z

Build failed.

ansible-tox-linters : FAILURE in 3m 47s

trishnaguha · 2019-08-12T14:09:14Z

Closed by PR ansible/ansible#60421

nxos_interfaces resource module

cb12d4a

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha changed the title ~~[WIP]nxos_interfaces resource module~~ [WIP]nxos_interfaces resource module POC Mar 11, 2019

trishnaguha force-pushed the interfaces_module_poc branch 3 times, most recently from e8925c4 to eb3560d Compare March 13, 2019 04:48

facts generation

4c8d98e

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from eb3560d to 4c8d98e Compare March 13, 2019 06:41

fix argument_spec instantiation

b04a7ab

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from 67666aa to b04a7ab Compare March 14, 2019 10:45

change operation keyword to state

77c8699

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch 2 times, most recently from 267fc31 to b041981 Compare March 18, 2019 15:04

Add facts module

e1ede04

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch 4 times, most recently from 77b51c5 to 6487949 Compare March 19, 2019 06:07

cosmetic changes for APIs

b42fbe9

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from 6487949 to b42fbe9 Compare March 19, 2019 06:12

Add utils

abc4839

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha mentioned this pull request Mar 19, 2019

Network Interfaces Resource Module #1

Open

execute_module API

ef011c6

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from d475218 to ef011c6 Compare March 19, 2019 12:27

privateip reviewed Mar 19, 2019

View reviewed changes

trishnaguha added 6 commits March 20, 2019 20:50

update ArgspecBase

2832b3c

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

ConfigBase class

3819eca

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

fix regex and state_deleted

a551055

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

move argspec to ansible_network_os dir

9d76427

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

standardize nomenclature

49241c6

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

set module params as want instead of creating key-value pairs again

2e45771

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from dc03bf8 to 2e45771 Compare March 22, 2019 07:27

ganeshrn reviewed Mar 25, 2019

View reviewed changes

trishnaguha added 2 commits March 27, 2019 12:31

Address review comment

cfc870c

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

remove ArgspecBase as it is not in use

2a5f6e8

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

NilashishC reviewed Apr 1, 2019

View reviewed changes

trishnaguha added 2 commits April 4, 2019 15:51

use get

91fa629

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

fix facts generation for before key in result

afafdcc

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from 4af98a0 to afafdcc Compare April 8, 2019 08:45

gather_network_resources attribute and facts update

522aa07

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

replace fix

4c0357c

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

update with facts builder

f96e588

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

trishnaguha force-pushed the interfaces_module_poc branch from b810dfa to f96e588 Compare May 23, 2019 10:24

trishnaguha changed the title ~~[WIP]nxos_interfaces resource module POC~~ nxos_interfaces resource module POC May 30, 2019

ikhan2010 changed the title ~~nxos_interfaces resource module POC~~ [nxos_interfaces] resource module POC Jul 15, 2019

trishnaguha closed this Aug 12, 2019

		from ansible.module_utils.six import iteritems


		class ArgspecBase(object):

Uh oh!

Conversation

trishnaguha commented Mar 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trishnaguha Mar 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trishnaguha Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trishnaguha Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NilashishC Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trishnaguha Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pabelanger commented Apr 25, 2019

Uh oh!

ansible-zuul Bot commented Apr 25, 2019

Uh oh!

ansible-zuul Bot commented Apr 30, 2019

Uh oh!

ansible-zuul Bot commented May 2, 2019

Uh oh!

ansible-zuul Bot commented May 20, 2019

Uh oh!

ansible-zuul Bot commented May 23, 2019

Uh oh!

trishnaguha commented Aug 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

trishnaguha Mar 21, 2019 •

edited

Loading

trishnaguha Mar 26, 2019 •

edited

Loading

trishnaguha Mar 26, 2019 •

edited

Loading

NilashishC Apr 1, 2019 •

edited

Loading

trishnaguha Apr 1, 2019 •

edited

Loading