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

[WIP][backward-conversion] Recognize templates #100 #152

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

[WIP][backward-conversion] Recognize templates #100 #152

wants to merge 1 commit into from

Conversation

atb00ker
Copy link
Member

@atb00ker atb00ker commented Apr 20, 2020 •
edited
Loading

Usage

temp1 = {
  "general": { "hostname": "device.example.com" }
}
# This dance of rendering the json is done to 
# remove all the keys with no value from json.
# Such keys are not desirable because they cause 
# some lists to appear not similar which may cause 
# data duplication
ren1 = OpenWrt(temp1).render()
native1 = OpenWrt(native=ren1)
temp1 = native1.json()

test = OpenWrt(native=open("device.example.com.tar.gz"))
print(test.distinct_native([temp1]))

Todo

  • Testing
  • Automated Testing
  • Documentation

Questions

  1. If someone sets some information, say, an interface in a template and apply it to the device and later remove that interface from luci, It will be resend by the controller and appear again on the device because it still exists in the template. How should be promote users from removing information from just their devices?

  2. In an array like section, say interfaces, if only one value of a member, say mtu of interface is changed, do I copy the entire interface or just the mtu value with interface name?
    Currently copying the entire thing because I feel it should be stored together. (Same for "radios" section)

Closes #100

@coveralls
Copy link

coveralls commented Apr 20, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-1.7%) to 98.198% when pulling 57856c6 on atb00ker:duplicate-check into cadb36b on openwisp:master.

nemesifier
nemesifier reviewed Jul 13, 2020
View reviewed changes
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

  1. please reword the question because I have not understood it 100%

  2. ideally we should store only what changes, this may imply to tweak the validation system, not sure (eg: if we only store one key and many other keys are required, how does that play? In theory if the validation happens on the merged configuration of config+templates, the validation should not complain).

@atb00ker
Copy link
Member Author

atb00ker commented Jul 13, 2020 •
edited
Loading

@nemesisdesign sorry, I read it and I haven't explained it enough. Basically, consider the case:

  1. User adds an interface in a template and adds the template in a device
  2. controller sends the interface to device and it gets added successfully.
  3. User deletes the interface from device (say by using ssh or luci)
  4. The new implementation of device sync in [WIP][backward-conversion] Recognize templates #100 #152 & openwisp-config would send the device information to controller
  5. controller compares the two configurations from device and controller.
  6. Sometime later, the controller will send the deleted interface again to the device and the user will be irritated by seeing the deleted interface and added.

Hence, I think for #100 we need to consider this case as well.

I hope I am able to explain the problem now.

nemesifier
nemesifier reviewed Jul 15, 2020
View reviewed changes
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

There's some problems:

user deletes something that is in a template on openwisp: we can't subtract something from a template currently
user deletes something that is in the device config on openwisp: we could find a way to delete it from openwisp as well, but it would not be consistent with the previous point
Moreover, implementing the mechanism which recognizes local changes in openwisp-config and sends it back to openwisp will require some thought.

I wouldn't attempt at doing everything in this phase.
Right now we should focus on getting the basics covered and iterate according to the feedback we get from users.

For these reasons, at the moment we should just assume OpenWISP will override the local configuration, which can be sent only during registration for example, is limited but it's something we can achieve soon and nonetheless a great step forward

@atb00ker atb00ker mentioned this pull request Dec 23, 2020
Draft
@atb00ker atb00ker mentioned this pull request Apr 9, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nemesifier nemesifier nemesifier left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[backward-conversion] Recognize templates
3 participants
@atb00ker @coveralls @nemesifier