search

scrapli / scrapli_community

scrapli community platforms
https://scrapli.github.io/scrapli_community/
MIT License
86 stars 26 forks source link

RFC - Transport Options w/ non-system transports #26

Closed carlmontanari closed 3 years ago

carlmontanari commented 3 years ago

If we pass a transport option in the SCRAPLI_PLATFORM like we do in the vrp platform here, and we arent using system transport, the underlying transport will blow up due to getting unused kwargs.

I think there are two reasonable "fixes" but curious what @AlexLardschneider and others think....

1) can just not pass transport args like this -- instead we can have the "full" class setup like the mikrotik routeros platform... in the init method prior to super'ing we can just pop the transport options out if the transport != system

2) we can just accept **kwargs in all the transports and ignore stuff we dont care about that way. this feels sorta messy but would also make it so we can have more divergence in what the transports support without breaking the "core" of scrapli things....

AFAIK the only affected platform is the vrp one so this is probably fairly low priority but something to consider/fix sooner rather than later!

Carl

AlexLardschneider commented 3 years ago

I'd definitely lean towards option 1 here. As you already said, accepting all **kwargs in all the transports and ignoring unnecessary ones feels a bit messy.

Since only the VRP platform is affected at the moment, updating it to use classes seems like a cleaner solution to me.

carlmontanari commented 3 years ago

Awesome, I'm game, thanks @AlexLardschneider I'll get a pr sometime today!

carlmontanari commented 3 years ago

Merged, hopefully I didn't bugger anything up :D