metent / uair

An extensible pomodoro timer
MIT License
93 stars 7 forks source link

`listen` should be an action of `uair`, not `uairctl` #18

Open jficz opened 10 months ago

jficz commented 10 months ago

Logically, the uairctl binary should be controlling existing instances out-of-band, like other -ctl tools commonly do.

The listen action breaks this paradigm by running in-band. Instead, the listen action should be done by the uair binary which is what starts the instance(s).

There are three non-exclusive workflows that come in mind:

  1. the listen action is performed by default if a running instance is detected
  2. the listen action starts a new instance if no running instance is detected
  3. a "start" and "listen" are exclusive actions, either resulting in error if the conditions for it are not met - this requires #17 to be implemented so that the error states can be handled properly in scripts
metent commented 10 months ago

The listen action breaks this paradigm by running in-band. Instead, the listen action should be done by the uair binary which is what starts the instance(s).

I disagree. The architecture of uair is a typical client-server one, where there is one uair acting as a centralized 'server' and the uairctl instances act as clients. The way uairctl listen works is that the uairctl instances starts listening to the same socket in which uair is sending the timing information. It is more or less how fetch command works, but listen just does it continuously until you manually interrupt.

uair is meant to be run as a daemon, maybe as a supervised process, if the user wants to run multiple synchronized instances. I should clarify this in the README.

Also, a shift in paradigm will require a lot of refactoring at this point.

jficz commented 10 months ago

... uair is meant to be run as a daemon, maybe as a supervised process, if the user wants to run multiple synchronized instances. I should clarify this in the README.

Fair enough, explained like this it makes more sense. The most confusing part for me was the *ctl part acting as a client more than a "controller" of sorts and the "daemon" part acting, in part, as a client on its own (as in it produces a directly consumable data on its stdout).

Refactoring would indeed be complicated and would break existing deployments quite a bit.

I'm happy with the explanation, up to you if you want to keep this open as a suggestion or not.

One thing though:

uair is meant to be run as a daemon, maybe as a supervised process,

In your opinion, would it make sense to ship a user systemd unit with uair to do just that? Or do you think this is more a task for packagers and distributors? I could write at least an example one to put in resources/, or perhaps under share/ to follow FHS a bit more closely? Not that it would be of any use in NixOS :)

metent commented 10 months ago

In your opinion, would it make sense to ship a user systemd unit with uair to do just that? Or do you think this is more a task for packagers and distributors? I could write at least an example one to put in resources/, or perhaps under share/ to follow FHS a bit more closely? Not that it would be of any use in NixOS :)

I think it's more of a task for package maintainers as init systems are OS and distro-dependent but it doesn't hurt to ship a service file with us. I'd be happy to accept any contributions. You can put a service file under the resources directory for now. I'll change the directory structure in the near future.