openflighthpc / flight-desktop

Manage interactive GUI desktop sessions
Eclipse Public License 2.0
0 stars 2 forks source link

Add a hooks directory to run post-verify scripts #38

Closed WilliamMcCumstie closed 3 years ago

WilliamMcCumstie commented 3 years ago

The new hooks directory supports optional post-verify scripts. This is to allow flight-desktop-restapi to be reloaded each time a new desktop is verified.

These scripts use the existing run_script infrastructure which execute through a bash shell. This means they do not need execute permissions. Some form of "admin only" scripts are required as not all users can restart the desktop-restapi service. This is achieve by checking if the user as read permission to the file.

benarmston commented 3 years ago

I've rebased onto latest master and made a few changes.

  1. The post-verify hook is now only called when a type transitions from unverified -> verified. The hook can be relied onto to indicate that there has been a change in verification of desktop types.
  2. Only a single post-verify hook script is supported. This makes output and directing users to logs easier.
  3. The hook now needs to be executable to be executed, instead of readable. It still needs to be a bash script.
WilliamMcCumstie commented 3 years ago

The changes look good in principle, just have one comment

  1. The post-verify hook is now only called when a type transitions from unverified -> verified. The hook can be relied onto to indicate that there has been a change in verification of desktop types.

Running post-verify after a change of state looks good, but it also needs to be ran if a type transitions from verified -> unverified.

Currently the post-verify hook is only used to restart the desktop-restapi, so the direction of the transition (verified <-> unverified) does not matter. This can be revisited if an when the direction becomes important.

benarmston commented 3 years ago

Running post-verify after a change of state looks good, but it also needs to be ran if a type transitions from verified -> unverified.

Yeah I mentioned that in the https://github.com/openflighthpc/flight-desktop/commit/1431cc1bf9a4ff36ec537909d5333aaf011be76f commit message. I was of the conclusion that it wasn't likely enough to be worth implementing. But on reflection it makes sense to do so now, whilst working on this. Thanks @WilliamMcCumstie