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

Add support for commands #91

Merged
merged 2 commits into from
Feb 11, 2018
Merged

Add support for commands #91

merged 2 commits into from
Feb 11, 2018

Conversation

mmihic
Copy link
Contributor

@mmihic mmihic commented Feb 10, 2018

Allows applications to register additional commands that can be run
directly from the jar, typically used for utility commands. For example,
an application might register commands allowing for interaction with the
service from the command line, supporting scripting of administrative or
operational tasks.

If no command is specified on the command line, the service is started
and blocks until terminated (consistent with the prior behavior of
running the jar)

Allows applications to register additional commands that can be run
directly from the jar, typically used for utility commands. For example,
an application might register commands allowing for interaction with the
service from the command line, supporting scripting of administrative or
operational tasks.

If no command is specified on the command line, the service is started
and blocks until terminated (consistent with the prior behavior of
running the jar)
Copy link
Contributor

@nbroyles nbroyles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool.

Might be worth nothing somewhere that you don't want to run commands in environments like Kubernetes as it will just cause the command to be run repeatedly, since Kubernetes expects whatever's running in its container to never terminate and will attempt to restart.

@mmihic
Copy link
Contributor Author

mmihic commented Feb 11, 2018

Yeah the point of this is to make it easier to provide administrative commands that you'd either run interactively on a VM, or would put into an admin script / cronjob

@mmihic mmihic closed this Feb 11, 2018
@mmihic mmihic reopened this Feb 11, 2018
@mmihic mmihic merged commit f85917d into master Feb 11, 2018
Copy link
Contributor

@ryanhall07 ryanhall07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always found the "split mode" for the SQ service container error prone. Every Component had to be careful what to install during UTILITY mode. For example, the initial version of the feeds module would spin up real feed consumers when somebody would run a command.

Do you plan on recreating the same thing in Misk?

try {
doRun(args)
} catch (e: CliException) {
log.info(e.message)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is info the right level?

@mmihic
Copy link
Contributor Author

mmihic commented Feb 12, 2018

Definitely not - this is intentionally designed to avoid that confusion. The problem in SC land is that the lifecycle of the application is controlled by the container, and the set of modules/services installed is global to the application rather than being specific to the command, so there was a) no way for a command to avoid starting services (the container did it automatically), b) no way for a command to control which services were installed (any dependencies declared on the application got installed, even if some weren't used by the command), and c) no way for a service to know whether it should let itself be started (a binary UTILITY mode is insufficient, since some commands might want some services and the service has no way of knowing).

The PR doesn't have any of that complexity - there are no modes, no enforced lifecycle, no automatic module/service installation, really no "container" per-se. There are just named commands that have parameters and a run method, with MiskApplication.run() being just a shell that parses the arguments to main() to determine which command to run and to populate that command's parameters with the values from the command line. Nothing is automatically installed by the container - if a command wants dependencies injected, it needs to explicitly specify the full set of modules/services to install. And nothing is automatically started - if a command wants to start services, it needs to explicitly do so in its run method. If no commands are installed or no arguments are passed, the application runs a default command that starts the server and waits for termination.

@ryanhall07
Copy link
Contributor

if a command wants dependencies injected, it needs to explicitly specify the full set of modules/services to install.

nice! thanks for the thorough explanation!

swankjesse pushed a commit that referenced this pull request Feb 17, 2018
rainecp pushed a commit that referenced this pull request Jul 24, 2023
@tso tso deleted the mmihic/cli branch September 18, 2023 15:00
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

Successfully merging this pull request may close these issues.

3 participants