implement process entrypoint and run cmd #89
Conversation
|
Happy to extract #88 into separate PR. regarding #83, the way I made it work is to silence std(out|err) output for all processes apart from the main one passed to note that I had to skip p.command.SetCmdArgs() since otherwise things would break in some cases (like running tests with |
|
note that I'm not well versed in |
|
run command can be tested using the config file: shell:
shell_command: "bash"
shell_argument: "-c"
processes:
main:
command: |
echo say hi
read -r in
echo $in
echo by
depends_on:
dep:
condition: process_started
availability:
exit_on_end: true
dep:
command: while true; do echo sleeping... && sleep 2; donerun the ./result/bin/process-compose run main -f process-compose.yamlthe ./result/bin/process-compose process logs -f dep |
|
and here's an example of how to configure a process to run without a shell wrapper: processes:
main:
no_shell: true
command: yarn
args:
- jest
- --watchWe could potentially default to > bash -c 'echo name: $0 arg1: $1' my-name my-arg
name: my-name arg1: my-argbut not sure if that would of use to anyone |
Not sure this is going to be a widely used notation :) |
|
Are there use cases where it only makes sense to run a process as part of the I tested the code (as is) with |
|
@adrian-gierakowski Thank you for that! |
👍
There might be some, but in my case it works both with |
could you post a minimal config I could use to reproduce? Or a nix expression? Thanks! |
version: "0.5"
log_level: debug
processes:
vim:
command: vim
args:
- compose.yaml
depends_on:
dep:
condition: process_started
dep:
command: while true; do echo sleeping... && sleep 0.5; doneTo reproduce:
I think this can be avoided if interactive processes are disabled when run with others. |
|
I guess vim doesn’t have non-interactive mode so it would only make sense to run it via I think we can leave it up to the user to choose the correct command to make their config work. We could also add a new flag which would disable the service when ran via |
I agree, We can start by letting the users manage the configuration if they want to skip the interactive processes. Since it's an edge case, I think it's a reasonable compromise. |
|
So, do you think adding the capability to pass additional args from cli to the main process ran with |
Yes, it looks good. There are a few things that I would have done differently, some small typos and no documentation, but for a WIP PR, it's in a great shape :) |
Ah yes, I’ll definitely add documentation. I’ll also double check for typos. Happy to make any further changes if you provide specific pointers. Thanks! |
src/cmd/run.go
Outdated
| // runCmd represents the up command | ||
| var runCmd = &cobra.Command{ | ||
| Use: "run [PROCESS]", | ||
| Short: "Run PROCESS in the forground (witn stdin, stdout, stderr attached), and and it's dependencies in the background", |
There was a problem hiding this comment.
And to be pedantic, "it's" as well 😄
https://www.merriam-webster.com/grammar/when-to-use-its-vs-its
There was a problem hiding this comment.
And witn -> with 🤣
|
@F1bonacc1 apologies for the long silence but I’ve been quite busy after coming back from holidays. I should have some time to work on this in about a week as I’ve got a couple of flights to take 😄 |
@adrian-gierakowski, |
adrian-gierakowski
force-pushed
the
implement-run-cmd
branch
from
6c84c82 to
29dba80
Compare
### process entrypoint `entrypoint` is an optional array of strings which can be set on the process config to override default shell (and shellArg) used to run the `command` When `entrypoint` is set and is non-empty, `command` can also be omitted. This is useful if one want's to: 1. use a different shell per process 2. run any other executable directly, without a shell wrapper ### `run` cmd Equivalent of [docker-compose run](https://docs.docker.com/engine/reference/commandline/compose_run/) which can be used to run a chosen process in the foreground (with std(in|out|err) attached), while it's dependencies are ran in the background (no logs printed to stdio). Deps can be disabled via --no-deps flag. Additional arguments can be passed to the process after `--` separator, for example: ```sh process-compose run main -- arg1 arg2 ``` The separator is necessary in order to distinguish process-compose flags from flags meant to be passed to the process. While the process is running, the logs of the deps can be inspected using the usual command: ``` process-compose process logs dep ``` The process being ran will have `availability.exit_on_end` set to `true` and manually setting it to `false` will have no effect.
adrian-gierakowski
force-pushed
the
implement-run-cmd
branch
from
29dba80 to
9aa5935
Compare
adrian-gierakowski
changed the title
|
@F1bonacc1 I believe I now got the desired behaviour implemented. I've updated PR description. Still missing documentation, but I'll add this once the implementation is finished (in case you have any further comments you'd like me to address). Not sure why tests are failing (haven't ran them before pushing), will have a look at it tomorrow. |
|
@F1bonacc1 not quite sure how to run the tests locally. There doesn't seem to be a way to do it via nix. And I also wasn't able to run the command which is used in github workflow from within |
|
@adrian-gierakowski, |
no luck: |
@adrian-gierakowski, git clone https://github.com/F1bonacc1/process-compose.git
cd process-compose
go get ./... # probably the missing step, please notice the 3 dots
make testrace |
|
still now luck: I did manage to run the tests by commenting out this bit: Line 41 in 5ec45e2 and running however some tests timed out due to lack of python3 in the sandbox. I think I should be able to deal with that. |
|
with the following patch I am able to run tests with: diff --git a/flake.nix b/flake.nix
index 5d17b81..c5de5a9 100644
--- a/flake.nix
+++ b/flake.nix
@@ -22,6 +22,14 @@
drv = self.packages."${system}".process-compose;
};
apps.default = self.apps."${system}".process-compose;
+ checks.default = self.packages."${system}".process-compose.overrideAttrs (prev: {
+ doCheck = true;
+ nativeBuildInputs =
+ prev.nativeBuildInputs
+ ++
+ (with nixpkgs.legacyPackages.${system}; [python3])
+ ;
+ });
}) |
|
@F1bonacc1 tests are fixed now do let me know if you have any comments, improvement suggestions or if you'd like me to add some tests, thanks! if you're happy with the implementation (or at least the interface), I'll start working on docs |
src/types/validators.go
Outdated
| } else { | ||
| message := fmt.Sprintf("Either command or entrypoint need to be non-empty (procces: %s)", name) | ||
| fmt.Println(message) | ||
| log.Fatal().Msg(message) |
There was a problem hiding this comment.
just realised that this is a breaking change, since previously empty command was allowed
There was a problem hiding this comment.
You are right, ignore the empty command for now. I will fix it in the Strict mode.
|
Everything looks great @adrian-gierakowski! |
this would be a breaking change as empty command was previously allowed also: log warning about both Command and Entrypoint being set to stderr to not interfere with downstream consumers of stdout
|
Kudos, SonarCloud Quality Gate passed!
|
| p.command = command.BuildCommandShellArg(p.shellConfig, p.getCommand()) | ||
| p.command = command.BuildCommand( | ||
| p.procConf.Executable, | ||
| append(p.procConf.Args, p.extraArgs...), |
There was a problem hiding this comment.
If p.procConf.Executable is bash and p.procConf.Args are []string{"-c", "echo Hi"}, p.extraArgs will be ignored.
Instead of that line, I call p.mergeExtraArgs():
func (p *Process) mergeExtraArgs() []string {
if len(p.extraArgs) == 0 {
return p.procConf.Args
}
tmp := make([]string, len(p.procConf.Args))
copy(tmp, p.procConf.Args)
if isStringDefined(p.procConf.Command) {
lastArg := p.procConf.Args[len(p.procConf.Args)-1]
lastArg += " " + strings.Join(p.extraArgs, " ")
return append(tmp[:len(tmp)-1], lastArg)
} else if len(p.procConf.Entrypoint) > 0 {
return append(tmp, p.extraArgs...)
}
return p.procConf.Args
}What do you think?
There was a problem hiding this comment.
Could you give an example of the final command you’d like executed?
There was a problem hiding this comment.
processes:
ls:
command: "ls"process-compose run ls -- /tmprun ls will ignore the extraArg /tmp
It worked well with entrypoint instead of command.
There was a problem hiding this comment.
This prints: args: 0 1 2
bash -c 'echo args: $0 $1 $2' 0 1 2 There was a problem hiding this comment.
Try:
bash -c "ls" /tmpvs.
bash -c "ls /tmp"
process entrypoint
entrypointis an optional array of strings which can be set on the process configto override default shell (and shellArg) used to run the
commandWhen
entrypointis set and is non-empty,commandcan also be omitted.This is useful if one want's to:
runcmdEquivalent of docker-compose run
which can be used to run a chosen process in the foreground (with std(in|out|err) attached),
while it's dependencies are ran in the background (no logs printed to stdio).
Deps can be disabled via --no-deps flag.
Additional arguments can be passed to the process after
--separator, for example:The separator is necessary in order to distinguish process-compose flags from flags
meant to be passed to the process.
While the process is running, the logs of the deps can be inspected using the
usual command:
The process being ran will have
availability.exit_on_endset totrueandmanually setting it to
falsewill have no effect.implements #88 and #83