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

[fix] Allow variables in mac address patterns in schema.py #239

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lpalgarvio
Copy link
Contributor

@lpalgarvio lpalgarvio commented Jun 20, 2022

  • Adds BLANK and VAR patterns
  • Patches MAC patterns
  • Removes usage of maxLength and minLength in mac and bssid properties

Closes #238

@lpalgarvio
Copy link
Contributor Author

tested as such:

root@openwisp:~# cp /tmp/schema.py /opt/openwisp2/env/lib/python3.7/site-packages/netjsonconfig/schema.py
root@openwisp:~# systemctl restart supervisor.service 
root@openwisp:~# supervisorctl restart openwisp2
openwisp2: stopped
openwisp2: started
root@openwisp:~# systemctl restart nginx.service

Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

Thank you contributing @lpalgarvio! I have left some suggestions below which will improve this PR.

netjsonconfig/schema.py Outdated Show resolved Hide resolved
netjsonconfig/schema.py Outdated Show resolved Hide resolved
netjsonconfig/schema.py Show resolved Hide resolved
@pandafy pandafy added this to To do (general) in OpenWISP Contributor's Board via automation Jul 5, 2022
@pandafy pandafy moved this from To do (general) to In progress in OpenWISP Contributor's Board Jul 5, 2022
@lpalgarvio
Copy link
Contributor Author

Thank you contributing @lpalgarvio! I have left some suggestions below which will improve this PR.

Cool, ill try these and update as soon as possible :)

@lpalgarvio
Copy link
Contributor Author

Thank you contributing @lpalgarvio! I have left some suggestions below which will improve this PR.

Sorry for the delay! A bit overloaded with OpenWrt projects and other things :)
Confirmed working! Added your commit suggestions.
Do you want me to squash these 3 commits?
@pandafy

@lpalgarvio lpalgarvio changed the title Fix usage of mac address patterns in schema.py to allow using variables (Closes #238) [fix] Fix usage of mac address patterns in schema.py to allow using variables (Closes #238) Jul 22, 2022
@lpalgarvio
Copy link
Contributor Author

Also maybe i should rename the commit to add "[fix]" to unbreak the CI checks

@pandafy pandafy changed the title [fix] Fix usage of mac address patterns in schema.py to allow using variables (Closes #238) [fix] Fix usage of mac address patterns in schema.py to allow using variables Jul 25, 2022
Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

Thanks for following up @lpalgarvio! I have tested your patch with openwisp-controller and these changes work as expected.

I have asked for one more improvement below. After that we should be able to merge this. Let us know if you'll be able to fix it.

Also maybe i should rename the commit to add "[fix]" to unbreak the CI checks

@lpalgarvio you are correct. The QA checks of OpenWISP requires the commit messages in a specific format. Please read the contribution guidelines. Don't worry about squashing the commits, we'll handle that when we merge the PR.

netjsonconfig/schema.py Outdated Show resolved Hide resolved
@lpalgarvio lpalgarvio changed the title [fix] Fix usage of mac address patterns in schema.py to allow using variables [fix] Allow variables in mac address patterns in schema.py Jul 26, 2022
@lpalgarvio
Copy link
Contributor Author

Thanks for following up @lpalgarvio! I have tested your patch with openwisp-controller and these changes work as expected.

I have asked for one more improvement below. After that we should be able to merge this. Let us know if you'll be able to fix it.

Also maybe i should rename the commit to add "[fix]" to unbreak the CI checks

@lpalgarvio you are correct. The QA checks of OpenWISP requires the commit messages in a specific format. Please read the contribution guidelines. Don't worry about squashing the commits, we'll handle that when we merge the PR.

fixed PR title, commit title and description, and because was already rebasing anyways to fix titles, squashed all commits

…enwisp#238)

- Adds BLANK and VAR patterns
- Patches MAC patterns
- Removes usage of maxLength and minLength in mac and bssid properties

Co-authored-by: Gagan Deep <the.one.above.all.titan@gmail.com>
Copy link
Member

@pandafy pandafy left a comment

Choose a reason for hiding this comment

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

Thank you very much for contributing @lpalgarvio! LGTM 👍🏼

Deferring merge to @nemesisdesign 😄

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

Successfully merging this pull request may close these issues.

Fix usage of mac address patterns in schema.py to allow using variables
2 participants