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

Raise an error to the user if the command parsed is blank #1405

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mike-weiner
Copy link

@mike-weiner mike-weiner commented Feb 8, 2025

Related to: #1399

weiner % kamal app exec --primary --env=TEST:1 '/usr/bin/echo :test'      
Get most recent version available as an image...
Launching command with version latest from new container...
  INFO [ab8c5ab1] Running docker run --rm --network kamal --env PORT="3000" --env TEST="1" --env /usr/bin/echo ="test" --log-opt max-size="10m" registry.hub.docker.com/<REDACTED>/<REDACTED>:latest  on <REDACTED>
  ERROR (SSHKit::Command::Failed): Exception while executing on host <REDACTED>: docker exit status: 125
docker stdout: Nothing written
docker stderr: docker: invalid reference format.
See 'docker run --help'.
option :env, aliases: "-e", type: :hash, desc: "Set environment variables for the command"

The problem described in #1399 appears to be a side effect of the env option being of type hash.

If the user's command to execute contains : and it appears after the first --env option in the command string, thor appears to try and (incorrectly) treat it as a key/value pair.

There should be a more informative error message returned to the user in this scenario. This problem is avoided altogether if the command to execute inside the container(s) is provided first followed by any options.

@mike-weiner mike-weiner changed the title Fail kamal app exec without a CMD Raise an error to the user if the command parsed is blank Feb 8, 2025
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.

1 participant