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

Warn about changed service files on dinitctl operations #403

Open
q66 opened this issue Oct 18, 2024 · 1 comment
Open

Warn about changed service files on dinitctl operations #403

q66 opened this issue Oct 18, 2024 · 1 comment

Comments

@q66
Copy link
Contributor

q66 commented Oct 18, 2024

When doing an operation on a service with dinitctl, it would be handy if dinitctl would warn when the service file has changed since the time it was first loaded. Since watching for file changes is a non-portable operation, this could be optional.

An approach I had in mind would be to use the Linux inotify interface (a BSD kqueue version could also exist, etc.) to watch service directories and keep the info attached to the service object; then pass it over the protocol when issuing FINDSERVICE or LOADSERVICE. This could be done by issuing a special event before returning the handle (as compliant protocol implementations are expected to ignore them at any point, this should be fine); or by attaching it to the FINDSERVICE/LOADSERVICE result. The latter is possibly more practical but the current protocol is restrictive in the sense it does not allow passing flags on the request, and it does not have enough space in the response to avoid breaking the existing protocol.

Either way, upon reception of this info, dinitctl could print a warning that the service file has changed and that one may want to reload.

Thoughts?

@davmac314
Copy link
Owner

I have thought about this before. I do agree that it would be good to give a warning in this situation. In terms of implementation, it would be easier just to keep the file timestamp (for when it was loaded), send that from the daemon to dinitctl via the protocol, and have dinitctl locate the file and check its current timestamp. The cost of doing that really is minimal and it's a lot less effort than watching a set of directories via inotify.

Whichever way it's done it's probably not going to be perfect: a service is potentially configured by multiple files, and the environment file (if any) also has an effect. But I guess a warning sometimes is better than never.

This could be done by issuing a special event before returning the handle [...] or by attaching it to the FINDSERVICE/LOADSERVICE result

Although it's more effort in the short term, I think the right way is probably to add a new version of FINDSERVICE/LOADSERVICE which also take a flags argument. (One flag could be to distinguish FIND vs LOAD, so it doesn't even need two new commands, just the one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants