trustd arguments: should we go back to subcommands? #185
Replies: 5 comments 1 reply
-
The two ideas I'm trying to keep in my brain simultaneous, without regard to how it's implemented are:
|
Beta Was this translation helpful? Give feedback.
-
Based on the assumption that the "old"
Multiple Commands (single binary) vs multiple binaries (single command)
I am more in favor of Multiple binaries (single command). Not sure how complex that might be in Rust, but coming from a Java world it was as easy as defining maven modules and defining module dependencies between them, then multiple executables can be created out of a single project I agree with @bobmcwhirter in regards of being able to serve the PM mode with a single command but that is just for demo purposes; for production environments we will need to execute each part of the app separately. Binary Argumentsjust a comment, not really an answer to the discussion's question: Trustify is a command line app that requires arguments to be passed to be initialization. While I don't see anything wrong there I just wanted to share that the more complex the app is the more arguments we will need to pass therefore the app should not rely a lot on arguments but more on "environment variables" or configuration files. Otherwise the arguments are too long that becomes a nightmare. |
Beta Was this translation helpful? Give feedback.
-
FWIW, I'm fine with multiple binaries. That's easy. And we can just remove PM mode might be most easily encapsulated within a shell script, e.g. ./trustify-db create
./trustify-api &
./trustify-ingest &
./trustify-ui & But I think more than a few quickly becomes unwieldy in docs and looks unprofessional, imo. |
Beta Was this translation helpful? Give feedback.
-
Powershell, Mac OS shell (bash 3), bash 4 (5?) … I think having multiple sub-commands makes sense. I also think having a default subcomment (pm mode) makes sense too. Going for multiple binaries will make the build more complex, and container creation too. It will lead to bigger to containers, or lead to more containers. Both is not ideal. So I think we should bring back sub-commands, and have a default "run it all for PM" sub-command as a default. |
Beta Was this translation helpful? Give feedback.
-
wfm |
Beta Was this translation helpful? Give feedback.
-
Somehow I've found myself in the midst of trying to address #135, #171 and #173 simultaneously. I'm attempting to do it -- poorly so far -- in #174. Consider this discussion a cry for help.
Here are the assumptions I was making:
trustd
to have no subcommands, i.e. there's never a reason for it to do significantly different things when you run it. It's a self-contained, data-driven REST API.Apparently, I was mistaken about the 2nd one, but I'll let Bob explain that below. 😄
My problems arose initially because the managed data wasn't being preserved across restarts of "PM mode". This was difficult to diagnose because the app was failing silently even though it was attempting to log error messages. This was because the
env_logger
wasn't initialized and our convention is to do that once-and-only-once inInfrastructure::run
which we currently only do in the REST server, hence #135.I don't like that trustd's args consist mostly of all of the server's args. This seems to be a cry for a subcommand, imo.
Further,
trustd
is doing a lot more work -- currently initializing a managed database -- than our oldtrust
command, which was really just a delegator of subcommands that each then assumed the responsibility ofInfrastructure
ing.So I think an argument can be made already for
trustd
to have 2 subcommands:trustd api
trustd db
This is the path of least resistance: we keep
Infrastructure
in the server only and close #135. We simplifytrustd
by moving all the db stuff out into a separate module (which should probably be done, regardless).To be clear, I'm not arguing for subcommands. I'd like for
trustd
to be a single, self-contained, data-driven app that provides a REST API. To do that right, I don't think it was enough to replicate only the fields of the server'sRun
struct toTrustd
. Most of the logic that used those fields should be moved as well. And that's the path of most resistance.I'm happy to follow that resistant path but not if the "no subcommands" mantra is wrong.
So, should we make
trustd
dumb, like the oldtrust
?Beta Was this translation helpful? Give feedback.
All reactions