Skip to content
This repository has been archived by the owner on Feb 10, 2018. It is now read-only.

add get_config #52

Merged
merged 6 commits into from
Sep 26, 2016
Merged

add get_config #52

merged 6 commits into from
Sep 26, 2016

Conversation

grizz
Copy link
Contributor

@grizz grizz commented Sep 26, 2016

No description provided.


# no get_cmd means it should mock the eznc get_config
else:
filename = 'get_config-' + options['database']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please insert all the options in the filename please?
e.g.:

'__'.join(['{0}_{1}'.format(k,v) for k,v in options.items()])

Eventually we can include also the filter at some point.


# no get_cmd means it should mock the eznc get_config
else:
filename = 'get_config-' + options['database']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And strip the hyphen

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use __ instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preferably yes

}

if retrieve in ('candidate', 'all'):
config = self.device.rpc.get_config(filter_xml=None, options=options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The candidate config would make sense if the user is in edit more - otherwise I think it should be empty.
You can introduce a new flag to check that, e.g.: pending_changes https://github.com/napalm-automation/napalm-iosxr/blob/develop/napalm_iosxr/iosxr.py that must be set in _load_candidate, commit_config and discard_config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with that, if I call get_config(retrieve='candidate') I'd expect something back and not an empty string. Happy to do it if that's what you guys have decided.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By definition the candidate config is only when in edit mode. It is equally true that the candidate config can be shared across multiple sessions, not limited to the current session only. So yes, I think your point makes sense to not restrict the gathering of the candidate config.

@grizz
Copy link
Contributor Author

grizz commented Sep 26, 2016

failed on if get_cmd: - which is used correctly, should I add a # noqa or why is it failing from a warning?

@dbarrosop
Copy link
Member

@grizz, that's not the error. However, if get_cmd doesn't seem to be the right way of using it. Check this thread: https://stackoverflow.com/questions/18583162/difference-between-if-obj-and-if-obj-is-not-none

In any case, the error is this:

======================================================================
ERROR: test_traceroute (TestJunOSDriver.TestGetterJunOSDriver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/src/napalm-base/napalm_base/test/base.py", line 457, in test_traceroute
    get_traceroute = self.device.traceroute(destination)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/napalm_junos/junos.py", line 1198, in traceroute
    rpc_reply = self.device._conn.rpc(traceroute_rpc)._NCElement__doc
  File "/home/travis/build/napalm-automation/napalm-junos/test/unit/TestJunOSDriver.py", line 162, in response
    rpc_reply = RPCReply(self._rpc.get_config(get_cmd=non_std_command))
  File "/home/travis/build/napalm-automation/napalm-junos/test/unit/TestJunOSDriver.py", line 137, in get_config
    filename = 'get_config-' + options['database']
KeyError: 'database'

@mirceaulinic
Copy link
Member

@grizz this is the error traceback:

===================================================
ERROR: test_traceroute (TestJunOSDriver.TestGetterJunOSDriver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/src/napalm-base/napalm_base/test/base.py", line 457, in test_traceroute
    get_traceroute = self.device.traceroute(destination)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/napalm_junos/junos.py", line 1198, in traceroute
    rpc_reply = self.device._conn.rpc(traceroute_rpc)._NCElement__doc
  File "/home/travis/build/napalm-automation/napalm-junos/test/unit/TestJunOSDriver.py", line 162, in response
    rpc_reply = RPCReply(self._rpc.get_config(get_cmd=non_std_command))
  File "/home/travis/build/napalm-automation/napalm-junos/test/unit/TestJunOSDriver.py", line 137, in get_config
    filename = 'get_config-' + options['database']
KeyError: 'database'

Fails in the traceroute method which does not send any options dictionary which is just a hack to reuse some of the existing code. Just make sure to include all the options in the filename.

@grizz
Copy link
Contributor Author

grizz commented Sep 26, 2016

Ahh right, not sure how I missed the traceback, will be fixed with the file rename.

And yes, in that case if get_cmd is correct because by default it's an empty str.

@mirceaulinic mirceaulinic added this to the 0.4.0 milestone Sep 26, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 84.773% when pulling 82b839c on grizz:get_config into 658339a on napalm-automation:develop.

@mirceaulinic mirceaulinic merged commit dcaf134 into napalm-automation:develop Sep 26, 2016
@grizz grizz deleted the get_config branch September 26, 2016 14:16
@mirceaulinic mirceaulinic mentioned this pull request Sep 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants