search

FNNDSC / ChRIS_ultron_backEnd

Backend for ChRIS
https://fnndsc.github.io/ChRIS_ultron_backEnd
MIT License
31 stars 100 forks source link

Simplifying command spec #372

Open jennydaman opened 2 years ago

jennydaman commented 2 years ago

Currently, the plugin spec requires plugins to define all of selfpath, selfexec, and execshell. This "spec" is clearly Python-biased, and even so it does not adhere to best practices:

  1. selfpath is not necessary: selfexec should be found in the PATH environment variable (OCI spec). Moreover, selfpath is usually managed by something such as pip, conda, ... so hard-coding it is bad practice.
  2. execshelldoes not make sense for binary applications. For interpreted languages, this should be configured by a shebang (#!). Again, this is managed by the language's package installer, such as pip.

I propose that the spec for ChRIS plugins simply be to give one string called command which represents the name of the executable. It should represent a file in PATH which has the executable bit set for all users. It can be either a plaintext file with a shebang, or a compiled binary application.

Related to https://github.com/FNNDSC/pman/issues/203

For backwards compatibility, perhaps command should instead be a list of strings instead where the first element is the name of an executable. Legacy plugin descriptions which specify selfpath, selfexec, and execshell can be accepted. These options are simply concatenated as ["{execshell}", "{selfpath}{selfexec}"]

beingnile commented 2 years ago

Hello @jennydaman, My name is Nile Okomo(GMT + 3), an outreachy applicant interested in being assigned this issue. Could I possibly work on it?

jennydaman commented 2 years ago

@beingnile you are welcome to take a stab. This would be a huge change. I'll lay out my plan of how this could be implemented, and it will be up to you whether you're up to the task.

This issue will be difficult to take up as it requires relatively in-depth knowledge of the ChRIS plugin specification. ChRIS plugins identify themselves to the ChRIS Store using the selfpath, selfexec, and execshell keys. This spec will be referred to as the "broken spec."

The API of pman was changed to accept a list[str] as entrypoint which represents the entirety of the information within the broken spec. This spec will be referred to as the "entrypoint spec." The entrypoint spec is simpler and better than the broken spec. However, changing CUBE to use the entrypoint spec would be very challenging.

  1. The ChRIS Store described plugins to CUBE using the broken spec, so the Store needs to be updated to the entrypoint spec first, then CUBE.
  2. Plugins describe themselves to the ChRIS Store using the broken spec, so the ChRIS Store must continue to support the broken spec (supporting a legacy but still prevalent API)
  3. CUBE database must be migrated, all plugins must be converted from broken spec to entrypoint spec.