From dad399f08ebd5792d8609ff5795e1b7b51514db9 Mon Sep 17 00:00:00 2001 From: Carl Montanari Date: Sat, 14 Aug 2021 11:26:23 -0700 Subject: [PATCH] do not force pty for netconf connections; flag to control this for system transport --- .../transport/plugins/paramiko/transport.py | 3 --- .../transport/plugins/ssh2/transport.py | 3 --- .../transport/plugins/system/transport.py | 16 +++++++++++----- tests/functional/driver/test_sync_driver.py | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/scrapli_netconf/transport/plugins/paramiko/transport.py b/scrapli_netconf/transport/plugins/paramiko/transport.py index d8fcda9..aa67038 100644 --- a/scrapli_netconf/transport/plugins/paramiko/transport.py +++ b/scrapli_netconf/transport/plugins/paramiko/transport.py @@ -44,7 +44,4 @@ def _open_channel(self) -> None: self.session_channel = self.session.open_session() self._set_timeout(self._base_transport_args.timeout_transport) - # unlike "normal" paramiko -- we do *not* need to enable the "shell" on the channel... - # we *do* still want it to be a pty though! - self.session_channel.get_pty() self.session_channel.invoke_subsystem("netconf") diff --git a/scrapli_netconf/transport/plugins/ssh2/transport.py b/scrapli_netconf/transport/plugins/ssh2/transport.py index c82957a..7299d3b 100644 --- a/scrapli_netconf/transport/plugins/ssh2/transport.py +++ b/scrapli_netconf/transport/plugins/ssh2/transport.py @@ -45,7 +45,4 @@ def _open_channel(self) -> None: raise ScrapliConnectionNotOpened self.session_channel = self.session.open_session() - # unlike "normal" ssh2 -- we do *not* need to enable the "shell" on the channel... - # we *do* still want it to be a pty though! - self.session_channel.pty() self.session_channel.subsystem("netconf") diff --git a/scrapli_netconf/transport/plugins/system/transport.py b/scrapli_netconf/transport/plugins/system/transport.py index bee821a..26e6050 100644 --- a/scrapli_netconf/transport/plugins/system/transport.py +++ b/scrapli_netconf/transport/plugins/system/transport.py @@ -20,11 +20,17 @@ def __init__( def _build_open_cmd(self) -> None: super()._build_open_cmd() - # adding `-tt` forces tty allocation which lets us send a string greater than 1024 chars; - # without this we are basically capped at 1024 chars and scrapli will/the connection will - # die. it *may* be possible to alter ptyprocess vendor'd code to add `stty -icanon` which - # should also have a similar affect, though this seems simpler. - self.open_cmd.extend(["-tt"]) + + # JunOS devices do not allocate pty on port 830 on some (all?) platforms, users can cause + # system transport to *not* force the pty (forcing pty is *default behavior*) by setting the + # transport arg `netconf_force_pty` to `False`. This defaults to `True` (forcing a pty) as + # that has been the default behavior for a while and seems to work in almost all cases, + # additionally without this -- in pytest (only in pytest for some reason?) output seems to + # come from devices out of order causing all the echo check logic to break... with this pty + # being forced that seems to never occur. Worth digging into more at some point... + if self._base_transport_args.transport_options.get("netconf_force_pty", True) is True: + self.open_cmd.append("-tt") + self.open_cmd.extend(["-s", "netconf"]) self.logger.debug(f"final open_cmd: {self.open_cmd}") diff --git a/tests/functional/driver/test_sync_driver.py b/tests/functional/driver/test_sync_driver.py index 7a60053..1d0eb88 100644 --- a/tests/functional/driver/test_sync_driver.py +++ b/tests/functional/driver/test_sync_driver.py @@ -164,7 +164,7 @@ def test_edit_config_and_discard(sync_conn, test_cases, config_replacer_dict, xm validate_filter = test_cases[device_type]["edit_config"]["validate_config_filter"] conn.open() - conn.get_config() + target = "candidate" response = conn.edit_config(config=config, target=target) assert isinstance(response, NetconfResponse)