-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
[WIP][backend] Added support for plugins. #120
base: master
Are you sure you want to change the base?
Conversation
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.
Great progress @R9295!
Let's start documenting this feature, I think we should do two things:
- dedicate a page to it
- mention it from basics and openwrt (which are the two most read pages), to increase chances users will see this
I can help you with these activities if you lay down the foundation for this.
def add_plugins(cls, plugins): | ||
for plugin in plugins: | ||
plugin.schema.pop('$schema', None) | ||
if getattr(cls, 'schema'): |
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.
as I was saying, we expect backend to always have a schema
, therefore if schema
is not present we should "fail early and loud"
if getattr(cls, 'schema'): | ||
cls.schema['properties'][plugin.key] = plugin.schema | ||
if getattr(cls, 'converters'): | ||
cls.converters.append(plugin.converter) |
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.
I think it's the same here, what's the use case of not having any converter declared? Wouldn't it just be wrong?
'uamsecret': 'asd123fghj' | ||
}, | ||
} | ||
OpenWrt.add_plugins([CoovaPlugin()]) |
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.
keep in mind that once you do this, the plugin will be enabled during all the tests that are called afterwards (even other test classes), this may have unintended consequences in the future, I think you should do two things:
- subclass the
OpenWrt
backend here in this file, redefine theschema
andconverters
attributes to explicitly use a copy (usedeepcopy
) ofOpenWrt.schema
andOpenWrt.converters
, then use the subclass here in the tests - add a
tearDown
method which cleans up any additional plugin every time a test method terminates execution
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.
@R9295 PS: please when you can, would be great if you could rebase on the current master and fix the flake8 warnings (I fixed the previous problem)
No description provided.