carlmontanari / scrapli

Fast, flexible, sync/async, Python 3.7+ screen scraping client specifically for network devices
https://carlmontanari.github.io/scrapli/
MIT License
575 stars 59 forks source link

Transport name validation only for core transport (to fix non-core transport loading) #329

Closed mvenditto closed 5 months ago

mvenditto commented 5 months ago

Description

As discussed in #327, custom/third-party transports are supported (see _load_non_core_transport_plugin) but because the transport name is checked against scrapli.transport.ASYNCIO_TRANSPORTS (in both AsyncScrapli and async_driver) async non-core transports cannot actually be loaded.

This PR modifies this behavior so that the ASYNCIO_TRANSPORTS checks occur only for core transports (scrapli.transport.CORE_TRANSPORTS).

Type of change

How Has This Been Tested?

Added test_transport_factory_non_core in test_base_base_driver to check that _transport_factory correctly loads the custom transport class (and arguments) from the appropriate plugin module.

Checklist:

mvenditto commented 5 months ago

A couple notes:

  1. I'm not super used with pytest so let me no if the ut I've added is not idiomatic enough or needs to be refactored!
  2. Not sure if this check should also be changed to account for possible ssh non-core transports?

Thanks!

carlmontanari commented 5 months ago

hey this looks great! will take a further look later in the week and we'll get this merged, thanks a bunch for the work on this!! grazie a te! (Scusami, vedo l'Italia nella tua bio e sto imparando l'italiano... allora... figured I could try some :))

mvenditto commented 5 months ago

hey this looks great! will take a further look later in the week and we'll get this merged, thanks a bunch for the work on this!! grazie a te! (Scusami, vedo l'Italia nella tua bio e sto imparando l'italiano... allora... figured I could try some :))

Il tuo italiano sembra buono! We may do the first bilingual PR review :) I noticed your surname too, if I may ask, are you by any chance of Italian descent?

carlmontanari commented 5 months ago

LGTM!

heh, Italian descent maybe a looong time ago :) (great great (great?) grandparents a napoli) -- ma amo l'Italia, il cibo, il vino, la storia antica etc.; tornare in due settimana -- prima volta a bologna, con mio moglie. siamo molto felici!

mvenditto commented 5 months ago

LGTM!

Perfetto, grazie!

tornare in due settimana -- prima volta a bologna, con mio moglie. siamo molto felici!

Emilia-Romagna is a great choice! it's the region where I live btw haha I hope you enjoy your trip!

P.S: if you ever end up in Forlì let me know, posso consigliarti un paio di posti dove si mangia bene!