load_merge_candidate() question

[decoupca]

For NXOS, if you call `device.load_merge_candidate(filename='merge.txt') more than once, you'll get a duplicate set of commands from merge.txt.

Example merge.txt:

!
hostname test
!

>>> device.load_merge_candidate(filename='merge.txt')
>>> device.load_merge_candidate(filename='merge.txt')
>>> print(device.merge_candidate)
!
hostname test
!
!
hostname test
!
>>>

Seems like at least for the filename arg, calling load_merge_candidate should replace any previously loaded merge candidate, not append. Before I open an issue/PR I wanted to see if this is intended behavior.

[ktbyers]

What happens if you try to load two different merge files (that have different changes)?

>>> device.load_merge_candidate(filename='merge1.txt')
>>> device.load_merge_candidate(filename='merge2.txt')
>>> print(device.merge_candidate)

[ktbyers]

The user didn't say to discard_config() on the pending change (to discard the pending candidate config), nor did you say commit_config() (to commit the initial candidate config), I am inclined to say that is the right behavior (though definitely open to arguments).

In other words, if you load a bunch of incremental merge changes one after the other they should all just go into the candidate config.

In the case of Cisco IOS and NX-OS (since the candidate config is not real) this can result in some strange things like duplicate configs, but the router will resolve those when the config is loaded. Also issues would potentially show up in the compare_config (though this compare_config on merge on IOS and NX-OS is challenging so this may or may not be true).

[decoupca]

The user didn't say to discard_config() on the pending change (to discard the pending candidate config), nor did you say commit_config() (to commit the initial candidate config), I am inclined to say that is the right behavior (though definitely open to arguments).

On the one hand, I agree. I haven't committed or discarded the config, which are core concepts in napalm.

OTOH, something about the semantic sound of load_merge_candidate(filename='whatever.txt') implies that I'm loading the entire merge candidate from a file, and somewhere in my head, I expect this would overwrite any previously loaded candidate. If the action were to append, I might expect append_merge_candidate(filename='whatever.txt'), although this is probably splitting hairs and I don't see a need to create a new method.

Either way I dont think this is a huge deal, I'm not strongly opposed to current behavior, it just seems slightly counterintuitive, but with the explanations above I'm good.