Skip to content
This repository has been archived by the owner on Mar 7, 2018. It is now read-only.

Add support for http and socket transports #65

Merged
merged 1 commit into from
Oct 24, 2016

Conversation

dbarrosop
Copy link
Member

@dbarrosop dbarrosop commented Oct 11, 2016

Fixes #64

@narJH27 would you mind testing this branch?

If it works I will document the optional arg in the official documentation.

@dbarrosop dbarrosop force-pushed the dbarrosop/add_support_for_different_transports branch from b058923 to db0b504 Compare October 11, 2016 08:40
@dbarrosop dbarrosop added this to the 0.4.1 milestone Oct 11, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 79.288% when pulling db0b504 on dbarrosop/add_support_for_different_transports into 1f0b976 on develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 79.288% when pulling db0b504 on dbarrosop/add_support_for_different_transports into 1f0b976 on develop.


if self.transport == 'https':
self.port = optional_args.get('port', 443)
elif self.transrpot == 'http':
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: self.transport instead of self.transrpot

self.port = optional_args.get('port', 443)
elif self.transrpot == 'http':
self.port = optional_args.get('port', 80)

self.enablepwd = optional_args.get('enable_password', '')

def open(self):
"""Implemantation of NAPALM method open."""
Copy link
Member

Choose a reason for hiding this comment

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

I also noticed this one Implemantation :)

@narJH27
Copy link
Contributor

narJH27 commented Oct 11, 2016

Also, the socket transport can only be used by a script on the box rather than a script trying to connect to the box from the outside. In my testing, my script could connect to the box using a UNIX socket using pyeapi.connect(<host>) as opposed to pyeapi.client.connect(). Maybe you can look into that?

@dbarrosop
Copy link
Member Author

Can you send your script and any relevant configuration? I just want to make sure I test your same exact conditions.

@narJH27
Copy link
Contributor

narJH27 commented Oct 17, 2016

@dbarrosop No relevant conditions as such. But my recollection of using pyeapi with unix sockets was as mentioned above. The script is nothing major, basically opening a connection to the box using the unix socket and calling one of napalm's methods, something like a get_bgp_neighbors_detail() which usually takes a few seconds to respond in the normal case.

@dbarrosop
Copy link
Member Author

@narJH27 I don't really understand what you are asking for with this:

the socket transport can only be used by a script on the box rather than a script trying to connect to the box from the outside

Unix sockets are local by nature, you can't connect from the outside unless they are exposed via a TCP port, which is what the http and https transports are for.

@narJH27
Copy link
Contributor

narJH27 commented Oct 18, 2016

@dbarrosop Apologies for using the phrase from the outside. What I meant was a local script connecting to the UNIX socket on the box rather than an outside connection using HTTP/HTTPS.

@dbarrosop
Copy link
Member Author

So this is done then. Merging.

@dbarrosop dbarrosop merged commit 637ebef into develop Oct 24, 2016
@dbarrosop dbarrosop modified the milestones: 0.4.0, 0.4.1 Oct 24, 2016
@mirceaulinic mirceaulinic deleted the dbarrosop/add_support_for_different_transports branch November 4, 2016 13:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants