-
Notifications
You must be signed in to change notification settings - Fork 91
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
Feature request reload plugins #73
Comments
Yes, it would be a good idea to make plugins loadable on the fly. The Backend module allows for reload and trigger of logics as well. |
For the time being, the Backend plugin implements the reload of logics in the same way as the CLI plugin does. It reloads only the Python code. It does not reload scheduling information from etc/logics.conf. Before starting to make plugins reloadable, I think its a good idea to do a complete implementation of the logic reload including the reread of the configuration. |
I have this feature already implemented in my local custom branch (reloading of logics, reloading of plugins - including reloading the configuration). But this implementation depends on some other features like putting configuration into different sources (files, database, ...). Since we also want to support different sources for configuration I could start to create one PR for this feature (support more different source formats, which will have the current supported one implemented). In the second step I could create a new PR including the reloading implementation. After all we could |
@bmxp I had reloading of plugins on my list for a long time. But like for features I was planning, @cstrassburg said he was working on it, so I stopped planing for those features. But since @cstrassburg has little/no time to spare, I restarted planning dynamic unload an load of plugins. I am even thinking in the direction of #27 (Checking plugin parameters against a metadata definition) |
Besides implementing the logics-api in the backend plugin I updated the cli plugin too. It uses the logics-api and has a new command to display extensive information about a logic. |
It is somewhat complicated and the plugins are mostly not written to be reloadable. I am not sure, we manage a reload ability for version 1.6, but we are working on it. |
As I recall of a recent gitter communication mostly plugins that utilize threads are a problem for a restart. There is already an entry |
Would it be an acceptable way to extend the stop() methods to do a more thorough cleanup, and perhaps set a flag self._stopped, so calling run() again could re-invoke init()? Or would a separate method restart() with possible cleanup and re-initialization be a better way? |
What do you mean by that?
That already exist -> self.alive
Not a good idea. Initialization is what should be done at load time. On the road to reloadable plugins, the first step would be stoppable plugins. Those plugins should be able to
|
Many plugins I have seen just these days don't care for "cleaning up", that is, closing connection, deleting schedulers, anything. All in all, when Python quits, it's all done, right? (...) So - as you said yourself in your last message - the first step should be to rework the stop() methods to be more thorough cleaning up the plugin environment.
self.alive would't tell if the plugin already had been running or was just initialized. But with a proper init/run/stop triple this shouldn't be necessary
Jein (is there something adequate in English? :D ) If I assume that restarting plugins (sooner or later) also means reloading item configuration "on the fly", then it might not be enough to re-run parse_item() for all items; also old item definitions and associations need to be removed beforehand. Also, maybe a plugin reload needs to load an (updated) command set or device definitions. So this needs a bit of thought. Split init into "pre-item-load" and "post-item-load", where post-item-load initialization need to be reversible (cleaning up 'registered' items -> also in shng's update_item() caller), or you just dump (unload) the whole plugin and load (import?) it again. This could be done without changing plugins, because it would need to be done by shng. Not sure if this is possible.
Agreed. Bad habit. Maybe we need to check this as a kind of quality control on accepting new plugins into develop? I know that at the moment only a few people can write to the shng repos (which should stay that way), but if there was a checklist to go along with the plugin, the author could beforehand check if all "required" marks are checked. Seeing that many plugin authors do not have much experience in programming, even less doing so to certain formal standards, this could be a way to start getting "better" code, meaning conforming to certain standard like "open connections in run, not before", "don't change items if self.alive == False", "close all connections / files on stop()" and so on. (Incidentally, these were also you last words... good to see we agree ;) ) |
Note: plugins changed due to lib.network adjustments should all be restartable now (no PR before 1.8) |
Any further thoughts / ideas on this topic?
I would be willing to start and try something in this direction, if we can agree on a way to go, on a kind of structure (see my comments above, pre-/post-item-init) |
We should consider to implement a way to start and stop plugins soon in a predefined way. With the new implementation of lib.network there are plugins in the wild that try to connect to devices that are frequently not in use like e.g. multimedia devices, tv sets, computers etc. |
Most of this is implemented (or prepared for implementation) in #477. So far, this didn't get much discussion. Admittedly, I've not yet progressed far with describing this outside the code... |
I am wondering if there is a list of problems to be solved to come to a state where everyone agrees "this feature is implemented". From what I can see in the discussion here it feels like "All plugins must be changed" and "we have to wait for dynamic item reloading", "besides: what about threads in plugins". This is meant by no means offensive, what I try to transport is: all these statements try to 'boil the ocean'. What I miss is the identification of the next 'baby-step' - and the request: "If we had this, then it would allow us to learn something from it for the next iteration." For me a roadmap could potentially look as follows (incomplete, and definitely partially matching some of the thoughts mentioned before):
Is there any better way of clustering this? I am getting used to the Scaled Agile Framework with Epic->Feature->PBI->Task in my professional context, which helps to divide big problems into doable actions for developers. Please do not understand this as an "I know better"-statement. I want to make my thought process transparent and I offer my support, if you provide a starting point for me ;-) |
Wow, lots of inputs ;)
When speaking of "make plugins reloadable" - no, not yet, as not all conditions and features are decided on. We are more or less agreed that before someone works on plugins to be restartable, one of the most important features will be dynamic item editing/reloading, so restartable plugins should be able to reload/reparse items. Lots of work in this context already has been done (I'm aware of #410, some work based on this issue #73, and #477), and waits to be assessed (which probably depends on me writing the docs beforehand... )
No, at least my plan is to adjust
See above. Both are - to a degree - interdependent because of structural changes to the modular setup.
Implemented by some plugins and working, with minor inadequacies mostly on shutdown (which are taken care of by Python)
No, not really. If you dare, try to follow the proposed changes in #477, I'm welcoming any feedback (aside from "write the f... docs" :)
This is my work-to-be-done, I guess - much of the code is already written but lacks documentation (description of work as well as description of goals, which would make up most of your roadmap).
If by modules you mean plugins, this is a problem I had already implemented in my md plugin project, which has been scrapped; still, the "way to go" is understood and implementable. However, I'd strongly advise against reloading arbitrary modules, only enabling to manually reload (recompile) single plugins on demand, mainly for developers. This is not meant to say "you're wrong", but I guess we look at the same problem from different angles, this is mine (as limited as my plannable available time for coding (or writing docs) at the moment...) |
Would you please be so kind to include a kind of |
Suspend() + activate(), isnt this the same as stop() + run()? At least in my plugins I had used the methods in this way, for exactly that purpose - a device that goes offline due to a hard power-off. |
I think it is not. suspend() means the plugin will neither take action nor respond but still has the ability to listen to the changes for items etc. Its just not querying actively. stop and run normally will also end the plugins thread I think which is not wanted |
If the plugin neither acts on item changes nor queries the associated device, what does it do in the meantime? I understand |
Imagine knx as readonly. It listens but does not take action. |
Hm... so for non-present network devices (media player e.g.), start/stop would be the way to go as @jentz1986 mentioned; and you want an additional "mode"? Should "probably be sent" be deterministic or based on - which - criteria? How long should item changes be stored/held back, if suspend takes a day or a week? (trying to image a use case... ) |
Start/stop(/restart) is implemented, adding and removing items to/from plugins and parsing/unparsing items, as far as the core/smartplugin class is concerned. Will open new issue to adjust plugins and track progress. @Morg42 Will also try to implement a suspend/resume function if precise instructions (what may/may not happen during suspend) are supplied - recommend opening a new issue for this @bmxp |
I wonder if someone was thinking about implementing a reload plugins action?
Currently the CLI module allows reloading logics. Wouldn't it be interesting having the same for plugins?
This feature would make it easier to update plugins and keep smarthomeNG core running.
The text was updated successfully, but these errors were encountered: