Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal of API for discussion #2

Closed
wants to merge 2 commits into from
Closed

Proposal of API for discussion #2

wants to merge 2 commits into from

Conversation

dbarrosop
Copy link
Member

@dbarrosop dbarrosop commented May 14, 2017

Mostly copy/pasting from napalm-base with tiny minor adjustments. Feel free to discuss in the code which parts we should change.

Also, I was thinking that instead of overriding methods, more specific drivers could implement an interface. For example:

class BaseNetworkDriver(object):
    ...
    def open(self):
        try:
            self._open()
        except Exception as e:
            raise Blah(orig_exc=e)

    def _open(self):
        raise NotImplementedError


class IOSDriver(BaseNetworkDriver):

    def _open(self):
        # code goes here

Or even decoupling napalm from the driver interface. For example:

class Napalm(object):
    """ This is the class users interact with."""

    def __init__(self, hostname, username, password, os_name, **kwargs):
        driver = get_network_driver(os_name)
        self.device = driver(hostname, username, password, **kwargs)
        ...

    def cli(self, commands):
        try:
            self.device.cli(commands)
        except Exception as e:
            raise UncaughtException(orig_exc=e)


class DriverInterface(object):
    """ This class simply defines/documents the interface. """

    def cli(self, commands):
        raise NotImplementedError


class IOSDriver(DriverInterface):
   """ This is a driver. """

    def cli(self, commands):
        # code goes here

Both approaches will give us more control and allow us to write wrappers to verify exceptions and put common code.

Opens a connection to the device.

Raises:
(we should define here which exception we want to raise and in which cases)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree on this.
I see an exceptions module below. Wouldn't we use the napalm-base exceptions?
We have this epic: napalm-automation/napalm-base#218 which is supposed to track some more exceptions to be introduced.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. This would basically replace the napalm-base itself...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants