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

Allow multiple changes over the same session #134

Merged
merged 2 commits into from
Feb 8, 2017

Conversation

mirceaulinic
Copy link
Member

@mirceaulinic mirceaulinic commented Feb 2, 2017

Closes #133

@mirceaulinic mirceaulinic added this to the 0.5.4 milestone Feb 2, 2017
@mirceaulinic
Copy link
Member Author

Ping @bewing - can you test this branch and cross-check that it works as expected?

@coveralls
Copy link

coveralls commented Feb 2, 2017

Coverage Status

Coverage decreased (-0.5%) to 79.725% when pulling 1ce1cea on mirceaulinic:ld_merge_sess into adee78d on napalm-automation:develop.

@mirceaulinic
Copy link
Member Author

Before:

>>> i.open()
>>> i.load_merge_candidate(config='ntp server 1.2.3.4')
>>> i.load_merge_candidate(config='ntp server 5.6.7.8')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "napalm_eos/eos.py", line 170, in load_merge_candidate
    self._load_config(filename, config, False)
  File "napalm_eos/eos.py", line 127, in _load_config
    raise SessionLockedException('Session is already in use by napalm')
napalm_base.exceptions.SessionLockedException: Session is already in use by napalm
>>>

Which is not consistent with other drivers (not sure about IOS).

After:

>>> i.open()
>>> i.load_merge_candidate(config='ntp server 1.2.3.4')
>>> i.load_merge_candidate(config='ntp server 5.6.7.8')
>>> print i.compare_config()
@@ -41,6 +41,8 @@
 ip name-server vrf default 8.8.8.8
 !
 ntp source Loopback0
+ntp server 1.2.3.4
+ntp server 5.6.7.8

@mirceaulinic
Copy link
Member Author

mirceaulinic commented Feb 3, 2017

1ce1cea prevents applying parallel changes within the same configuration session.

@bewing
Copy link
Member

bewing commented Feb 3, 2017

Can confirm that this resolves saltstack/salt#39149

root@saltmaster:/srv/salt# salt veos1 state.sls router.ntp
veos1:
----------
          ID: cf_ntp
    Function: netntp.managed
      Result: True
     Comment:
     Started: 06:42:24.061354
    Duration: 5563.203 ms
     Changes:
              ----------
              servers:
                  ----------
                  added:
                      - 192.168.0.1
                  removed:
                      - 10.194.2.17

Summary for veos1
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   5.563 s

@mirceaulinic
Copy link
Member Author

Thanks for confirmation @bewing!

Anyway, the scope is not to solve that particular case, but more to assure consistency across napalm drivers.

@dbarrosop
Copy link
Member

Nice!

@mirceaulinic mirceaulinic merged commit 837722a into napalm-automation:develop Feb 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants