-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
openwisp_controller.connection module (SSH connections) #31
Conversation
On Debian Stretch, I additionally needed to install
to I can create a PR with updated README.rst and settings.py if you want. |
@okraits not sure travis would work with `SPATIALITE_LIBRARY_PATH = 'mod_spatialite' but we can try |
@nemesisdesign So should I come up with a PR for the dependencies and settings.py? Edit: there you are: #33 |
03b2772
to
3c5568c
Compare
5420a2d
to
f3f8334
Compare
I just rebased this branch on the current master. |
f3f8334
to
cbecd15
Compare
279fffe
to
55a168d
Compare
@okraits on the feature side it's basically ready, unfortunately there's no nice widget to enter the credentials (it's a raw text field which expects json syntax now) but shouldn't be hard to add it later, especially once this is ready: openwisp/django-jsonschema-widget#1) in the next weeks I count on adding the missing tests. |
|
||
|
||
class Ssh(object): | ||
schema = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@atb00ker this is the schema for SSH credentials. In the future we may have other ways to access devices. For example, OpenWrt
has an HTTP-RPC API served by ubus
. Ubiquiti AirOS also has some sort o HTTP access that allows to update the config via HTTP calls (not sure if they have an API but surely calls to their web interface can be simulated).
That's where we want to get at some point with openwisp/netjsonconfig#91
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems great. I see a lot of things i need to learn about and i'll start with reading the same.
Thanks. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found a small typo here:
print('# Previus command failed, aborting upgrade...') |
|
||
Available update strategies. An update strategy is a subclass of a | ||
connector class which defines an ``update_config`` method which is | ||
in charge of updating the configuratio of the device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing char: configuration
a72b423
to
0d2864f
Compare
@nemesisdesign Please can you review this when you have time? I will love to apply corrections if any exist |
out = '' | ||
while stdout.channel.recv_ready(): | ||
out += str(stdout.channel.recv(1024)) | ||
self.assertIn('openwisp-config restarted', out) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're executing an echo on the mock ssh server and asserting the output, for what purpose?
We need to mock the command /etc/init.d/openwisp-config restart
so when that command is executed in the mock ssh server it print that output, so we can then capture the output and run an assertion on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I myself was not convinced about this test. I was running from executing this comment on the server because from your last comment, you said this command was suppose to be executed during config.full_clean()
and config.save()
.
will try to refine it. Thanks
visible=[data['cred1'].name + ' (SSH)'], | ||
hidden=[data['cred2'].name + ' (SSH)', data['cred3_inactive']], | ||
select_widget=True | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this test being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was failing with Python 2.7 on travis, so i removed to deal with the most important issue before coming back to it
593b9e3
to
54f2480
Compare
54f2480
to
8900958
Compare
@nemesisdesign please can you review this, have been trying to make the test pass for |
from https://realpython.com/python-mock-library/ , python2.7 doesn't know |
WIP, refers to #30