search

OpenZWave / qt-openzwave

QT5 Wrapper for OpenZWave
GNU Lesser General Public License v3.0
105 stars 30 forks source link

Update README.md #109

Closed kpine closed 4 years ago

kpine commented 4 years ago

Unsolicited update to README file. :sweat_smile:

Includes a few cosmetic fixes.

The docker section has been re-organized and the examples have been reformatted with additional explanations, in the hope of making it easier for new users.

Obviously a lot of opinionated changes, so take what you like, change whatever. A live version of the file can be viewed from the diff controls (... -> View File).

Fishwaldo commented 4 years ago

Looks good. I was going to solicit feedback if we should run the standard ozwdaemon under supervisord or not? Mainly I was thinking of this from the log perspective. (Instead of docker logs)

kpine commented 4 years ago

I guess that's a philosophical question... The s6-overlay project has a good topic on that very question.

It's arguable that the standalone container shouldn't have a supervisor at all, and the application should manage the logs. But it may be more pragmatic to let a supervisor handle all that, if it's easiest.

If you go with a supervisor, I don't think it should manage the ozdaemon process at all (see that s6-overlay topic). If ozwdaemon crashes or even exits normally, the supervisor should also exit and let the admin's process (docker restart policy or whatever) handle it.

I can't recall what the all-in-one image is doing, but I think it should work the same way. If ozw-admin exits or crashes, it should probably just be restarted. But if ozwdaemon exits or crashes it should exit the entire container.

BTW, s6-overlay seems to be a popular supervisor for Docker containers. HA is using it. Have you looked at it before? Maybe it's a little more complicated to use than supervisord, but it's got a very logical design to it. It might facilitate the possibility of more features, such as being able to run as non-root.

Fishwaldo commented 4 years ago

I'll have a look at s6 - Thanks for PR though!