Polish interaction between scrapli connection and scrapli-cfg conn

[dmfigol]

Currently it is slightly confusing, since the only "real" connection is scrapli one, however we have double open and then close on ScrapliCfgPlatform side. It becomes even more confusing when the intention is to use it with a context manager. E.g. currently I am using:

    with Scrapli(**device) as conn:
        cfg_conn = ScrapliCfg(conn=conn)  # type: ignore
        cfg_conn.open()
        rendered_config = cfg_conn.render_substituted_config(
            config_template=cfg, substitutes=[("bgp", BGP_PATTERN)]
        )
        cfg_conn.load_config(config=rendered_config, replace=True)
        diff = cfg_conn.diff_config()
        print(diff.side_by_side_diff)
        print(diff.unified_diff)
        cfg_response = cfg_conn.commit_config()
        cfg_response.raise_for_status()

I think it would be possible to remove open() and close() , as well as __enter__ and __exit__ methods on ScrapliCfgPlatform and have some special commands that needs to be sent moved to a separate method (e.g. prepare). A method like this could implicitly be called when we call render_substituted_config or load_config (not sure about this, just an idea).

[carlmontanari]

I dont think I would remove enter/exit -- I was kinda thinking about (but haven't documented it like this :shame:) using it like this:

import asyncio

from scrapli.driver.core import AsyncIOSXEDriver
from scrapli_cfg.platform.core.cisco_iosxe import AsyncScrapliCfgIOSXE

DEVICE = {
    "host": "localhost",
    "port": 21022,
    "comms_ansi": True,
    "auth_strict_key": False,
    "auth_username": "vrnetlab",
    "auth_password": "VR-netlab9",
    "auth_secondary": "VR-netlab9",
    "transport": "asyncssh",
}


async def main():
    """blah"""
    async with AsyncIOSXEDriver(**DEVICE) as conn, AsyncScrapliCfgIOSXE(
        conn=conn, preserve_connection=True
    ) as cfg_conn:
        cfg = await cfg_conn.get_config()
        print(cfg.result.splitlines()[:5])


if __name__ == "__main__":
    asyncio.get_event_loop().run_until_complete(main())

This brings up another thing I know we've chatted about -- about making that preserve_connection default (or renaming it because still pre release so we can do w/e we want)... but thats probably a different topic.

I also envision using it kinda like this:

"""blah"""
import asyncio

from scrapli.driver.core import AsyncIOSXEDriver
from scrapli_cfg.platform.core.cisco_iosxe import AsyncScrapliCfgIOSXE

DEVICE = {
    "host": "localhost",
    "port": 21022,
    "comms_ansi": True,
    "auth_strict_key": False,
    "auth_username": "vrnetlab",
    "auth_password": "VR-netlab9",
    "auth_secondary": "VR-netlab9",
    "transport": "asyncssh",
}


async def main():
    """blah""" 
    conn = AsyncIOSXEDriver(**DEVICE)
    # doin stuff, maybe conn comes from some other function/place in your code

    async with AsyncScrapliCfgIOSXE(conn=conn, preserve_connection=True):
        cfg = await cfg_conn.get_config()
        print(cfg.result.splitlines()[:5])

    # go back to doing w/e you wanna do w/ the "normal" connection preserved and chillin



if __name__ == "__main__":
    asyncio.get_event_loop().run_until_complete(main())

I dont think want to have the "open-y" things happening at any time other than an "open" (though perhaps "open" should be renamed as you point out) -- the reason for this is that "open" in scrapli-cfg context actually does things in that it sends configs (ex: no logging monitor for iosxe) as opposed to "normal" scrapli that just disables paging or w/e but thats not an actual "config" nor is it persistent. So basically I want that to only happen at very explicit/obvious times. Maybe thats not worth caring about? I dunno. For now at least thats kinda the thought for the "open" bits beings no explicit.

All of that said -- I think you are right that it is probably overly confusing.... so either something needs to change or docs need to get better (which needs to happen anyway, just kinda early days for this still at the moment).

Whatchya think?

[carlmontanari]

made some changes and updated docs/examples/readme to reflect this.... the cliff notes version:

• no more open/close -- as we discussed offline that was confusing since the connection wasn't really being opened/closed necessarily -- there is now prepare/cleanup
• previous behavior of scrapli-cfg "managing" the scrapli connection (opening/closing it) can still be achieved w/ the dedicated_connection=True argument -- when in "dedicated connection" mode we assume you will not be using the scrapli connection before/after the context manager (or manually calling the prepare/cleanup methods) so we will actually open/close the underlying scrapli connection -- this is not default behavior now
• context manager still exists and calls the prepare/cleanup methods for you -- but as noted above this by default will not touch the underlying scrapli connection. if the scrapli connection is not open scrapli-cfg will raise an exception
• the on_open bits have been replaced by a more suitably named on_prepare and the default functions for this have all been removed -- thereby eliminating the "magic" :D (also this makes cfg behave more similarly to napalm for better or for worse). users can pass a callable to on_prepare to do things like disable console logging and such
• semi-related -- the ignore_version argument was added -- for now this is not super useful but the future plans include having scrapli-cfg check target device versions and checking if a method is permissible for that version (ex: eos load config requires >=4.14 eos). if ignore_version is False (default) and a user does not use a context manager, but does not call the prepare method, then an exception will be raised indicating the user has not called prepare

Phew.... I think this conversation has made things better.... hopefully you agree! still have a few months until the first "real" (non pre) release, so defo time to dial things in!