[nxos_interfaces] resource module POC #2
Conversation
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
e8925c4 to
eb3560d
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
eb3560d to
4c8d98e
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
67666aa to
b04a7ab
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
267fc31 to
b041981
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
77b51c5 to
6487949
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
6487949 to
b42fbe9
Compare
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
d475218 to
ef011c6
Compare
module_utils/argspec/base.py
Outdated
| from ansible.module_utils.six import iteritems | ||
|
|
||
|
|
||
| class ArgspecBase(object): |
There was a problem hiding this comment.
If you are planning to provide direct attribute access to dict values, you should implement __delattr__ as well.
There was a problem hiding this comment.
How will other module arguments be implemented? required_if, required_together, etc
There was a problem hiding this comment.
@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',))]
module_utils/argspec/base.py
Outdated
| if key in self.argument_spec: | ||
| setattr(self, key, value) | ||
|
|
||
| def __getattr__(self, key): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Attributes that begin with _ should only be checked against the internal structure not the instance variable, imho
module_utils/argspec/base.py
Outdated
| argument_spec = {} | ||
|
|
||
| def __init__(self, **kwargs): | ||
| self.values = {} |
There was a problem hiding this comment.
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
dc03bf8 to
2e45771
Compare
| class FactsArgs(ArgspecBase): | ||
|
|
||
| argument_spec = { | ||
| 'gather_subset': dict(default=['all'], type='list') |
There was a problem hiding this comment.
Can we add individual supported keys, so that it is clear to the user which all option values it can support?
| self._module = module | ||
| self._connection = self._get_connection() | ||
|
|
||
| def _get_connection(self): |
There was a problem hiding this comment.
This API can be moved to the base class?
There was a problem hiding this comment.
@ganeshrn which base class are you referring to?
| class Facts(FactsArgs, FactsBase): | ||
|
|
||
| VALID_SUBSETS = [ | ||
| 'net_configuration_interfaces', |
There was a problem hiding this comment.
Add this value in the argspec for class FactsArgs(ArgspecBase)
'gather_subset': dict(default=['all'], choices=['net_configuration_interfaces', 'all'], type='list')
module_utils/network/argspec/base.py
Outdated
| @@ -0,0 +1,4 @@ | |||
| class ArgspecBase(object): | |||
There was a problem hiding this comment.
What is the purpose of this class?
There was a problem hiding this comment.
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>
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
| :rtype: A text string | ||
| :returns: A text string if match is found | ||
| """ | ||
| match = re.search(r'%s (.+)(\n|$)' % arg, cfg, re.M) |
There was a problem hiding this comment.
@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>
4af98a0 to
afafdcc
Compare
|
recheck |
|
Build failed.
|
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
|
Build failed.
|
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
|
Build failed.
|
|
Build failed.
|
Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
b810dfa to
f96e588
Compare
|
Build failed.
|
|
Closed by PR ansible/ansible#60421 |
Signed-off-by: Trishna Guha trishnaguha17@gmail.com