This repository was archived by the owner on Jul 15, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 15
adding merge_config support #2
Open
fooelisa
wants to merge
20
commits into
spotify:master
Choose a base branch
from
fooelisa:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
e20cf6e
working on merge_config
62e02d5
adding merge_config
a057d9e
adding merge_config
b089ab3
adding a few changes as per the commentary on pull/2
07cd5d6
david wins
f32c7b7
now where we agreed on 120 char width...
3207c5d
compare_config -> compare_replace_config in tests and doc
25605f6
Merge pull request #4 from dbarrosop/master
dbarrosop 19f310d
Merge pull request #5 from dbarrosop/master
dbarrosop 9eb3f45
ok ok, i give up
c088113
until arista supports commit and we see how to implement that... bye
3d968eb
working on merge_config
f21e79a
adding merge_config
73c3ceb
adding merge_config
fb8113e
adding a few changes as per the commentary on pull/2
99dbcf4
david wins
445bb47
now where we agreed on 120 char width...
5c3d2ae
until arista supports commit and we see how to implement that... bye
d554cfb
until arista supports commit and we see how to implement that... bye
d90044d
until arista supports commit and we see how to implement that... bye
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should actually remove that option and make everybody use SSL always. Why? Napalm doesn´t support non-ssl protocols (https is kind of hardcoded for EOS and netconf and expect uses ssh as transport so...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that for a brief moment yesterday, but I feel like the device libs (eos same as iosxr for that matter) are closer to the actual hardware and are ok to support vendor specific things, where napalm should unify them into one common interface (I would not include it there). But if someone uses pyEOS standalone and has https trouble - I find it ok to leave this option in for that use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, disregard. I was thinking in napalm. I personally don´t think we should support/provide this plugin at all. I put it there as a proof of concept but I am not planning to use it so I don´t want to spend time or effort in working on that. But if you want to... feel free ; )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not, if it's already there, as long as it is consistent :) we can always delete it in the future, if we get sick of supporting this