-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
[feature] Added "auth_nocache" option to OpenVPN backend #215
Conversation
42d99b9
to
305e697
Compare
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.
@aagman945 this PR will need some more work before merging.
We will need to update the auto_client
function of OpenVpn class to add auth_nocache
field.
netjsonconfig/netjsonconfig/backends/openvpn/openvpn.py
Lines 23 to 34 in a90813c
@classmethod | |
def auto_client( | |
cls, | |
host, | |
server, | |
ca_path=None, | |
ca_contents=None, | |
cert_path=None, | |
cert_contents=None, | |
key_path=None, | |
key_contents=None, | |
): |
Adding auth_nocache
in the copy_keys
list should do it.
We will also need to update tests after
making this change. In the tests, set the value of auth_nocache
to True
.
The documentation needs to be updated as well to reflect addition of this field.
@pandafy should I update all the tests, or just |
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.
Good work @aagman945, everything looks good to me. I have tested this with openwisp-controller as well.
The only problem I see is with the commit message. The commit message does not reference the issue. Read the commit style guideline here https://openwisp.io/docs/developer/contributing.html#how-to-commit-your-changes-properly
Also, can you squash you commits and rebase you PR on the latest master?
|
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.
@aagman945 the build is failing!
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.
LGTM! 👍🏼
@aagman945 the first word of the commit description should be capitalised ("Closes" and not "closes"). This time, we will update the description while merging. But if you are eager to continue contributing to OpenWISP, I ask you to please take care of these small details from next time.
Thanks for contributing!
Deferring merge to @nemesisdesign 😄
I'll take care from next time. |
Added
auth_nocache
to OpenVPNCloses #177