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.

Sign up for GitHub

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

Jump to bottom

[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
Open

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

wants to merge 1 commit into from

Conversation

lpalgarvio
Copy link
Contributor

@lpalgarvio lpalgarvio commented Jun 20, 2022 •
edited by pandafy
Loading

  • 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

pandafy
pandafy requested changes Jul 5, 2022
View reviewed changes
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
@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
pandafy
pandafy requested changes Jul 25, 2022
View reviewed changes
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

@lpalgarvio @pandafy
[fix] Allow variables in mac address patterns in schema.py (Closes op…
a446f78 
…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 <[email protected]>
@lpalgarvio lpalgarvio requested a review from pandafy July 26, 2022 02:28
pandafy
pandafy approved these changes Jul 26, 2022
View reviewed changes
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
Reviewers

@pandafy pandafy pandafy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
OpenWISP Contributor's Board
Status: In progress
Milestone
No milestone
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
@lpalgarvio @pandafy