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

[backends] Add a way to easily extend Backends #112

Open
nemesifier opened this issue Jul 20, 2018 · 4 comments
Open

[backends] Add a way to easily extend Backends #112

nemesifier opened this issue Jul 20, 2018 · 4 comments

Comments

@nemesifier
Copy link
Member

A very common scenario to create a custom backend is to add support for a new OpenWRT package.

Contributors may then want to include this package in the main code, but we cannot add all of these to the master branch for several reasons: negatively affecting performance in the OpenWISP web UI (schemas are loaded with HTTP requests and rendered with javascript, the more complex a schema is the more time it will be needed to download it and render the UI for it), increasing complexity.

We should start thinking about a way to dynamically and declaratively extend a backend so that a new package can be supported, or something along these lines.

@edoput
Copy link
Contributor

edoput commented Jul 20, 2018

A simple solution would be to extend converters as we did wiht the backend class

# myconverter/__init__.py
from netjsonconfig import BaseConverter

class MyConverter(BaseConverter):
    ...

# setup.py
...

setup(
    name='my_converter',
    ...,
    entry_points={                                                                                                                              
          'netjsonconfig.openwrt.converter': [                                                                                                             
              'my_converter=my_converter.__init__:MyConverter',                                                                                         
          ]                                                                                                                                       
    },
    ...,
)

With this approach we could have a backend probe for converters at runtime.

I can propose a patch somewhere in the next two week if needed otherwise I'm here to help
to make it possible

@edoput
Copy link
Contributor

edoput commented Jul 20, 2018

Another problem is that we would have to merge their json schema with the backend one; this may be a problem we previously encountered.

Otherwise we could push the schema validation to converters instead

@nemesifier
Copy link
Member Author

@edoput not everyone that wants to slightly extend netjsonconfig may be willing to put the effort to prepare a python package (imagine someone that installs openwisp and simply wants to add some custom logic to it).
I was thinking of introducing some more object oriented logic in the library itself to extend the backends.. very rough example:

from netjsonconfig import OpenWrt
from .mycoolproject import MyCoolRenderer, schema

OpenWrt.add(renderers=[MyCoolRenderer],
            schema=schema)

Or maybe we could even allow renderers to declare a schema which could be automatically merged by netjsonconfig.

This would be very interesting because it would also allow us to have some additional renderers that are used often enough to be published in the main repo and documented in our official docs but not so much to be activated by default, so we may provide clear documentation on how to activate these and we may even enable ansible-openwisp2 to do this very easily.

That way we can provide some additional default renderers for things like DDNS (#111), OLSRd, OLSRd2, Bmx7, etc..

Do you start seeing the possibilities I see?

@edoput
Copy link
Contributor

edoput commented Jul 26, 2018

not everyone that wants to slightly extend netjsonconfig may be willing to put the effort to prepare a python package

this is not true. making a python package is as simple as adding a setup.py and another folder in your repo, it does not require anything else.

I was thinking of introducing some more object oriented logic in the library itself to extend the backends.

I'm not against extending the library but logic here is the wrong word. logic implies that there is control (inside netjsonconfig) on what gets loaded but in a library this is unfortunately bad thing, when
you want control you implement an ACL and this is out of scope of netjson rendering

Maybe we need another layer between netjsonconfig and django but this may be too much work

OpenWrt.add(renderers=[MyCoolRenderer], schema=schema)

this is not nice, it implies that now I have to track the new backend instance or that netjsonconfig store
its state of loaded/unloaded plugins somewhere and it needs to be consistend over time (upgrades, insertion, removal, change of python path...) , the following is nicer and already working, without any need to add code. If I need to extend the capabilities of netjsonconfig as a library this is how I would do it before integrating with whatever I need to.

b = OpenWrt(...)
b.converters.append(MyConverter)
b.render()

Or maybe we could even allow renderers to declare a schema which could be automatically merged by netjsonconfig.

I support the idea to split the schema in smaller modules but again we should fix schema merging before

This would be very interesting because it would also allow us to have some additional renderers that are used often enough to be published in the main repo and documented in our official docs but not so much to be activated by default

python packages namespace are what you would use to solve this problem, we can declare the namespace netjsonconfig and make additional packages declare they are part of a namespace.
they get installed only when the user asks for it but they are stored in the same codebase.

so we may provide clear documentation on how to activate these and we may even enable ansible-openwisp2 to do this very easily.

yeah but the burden of deciding which extensions should be loaded should be placed in the openwisp django settings file. I figure that an exclude list can be passed to netjsonconfig.load_backends or netjsonconfig.load_extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do (Python & Django)
Status: To do (Device management)
Development

No branches or pull requests

2 participants