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

Allow modal shell -c to run a command #2333

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Allow modal shell -c to run a command #2333

merged 1 commit into from
Oct 14, 2024

Conversation

ekzhang
Copy link
Member

@ekzhang ekzhang commented Oct 9, 2024

This is analogous to python -c "print(3 + 4)", or also bash -c "echo hello".

Changelog

  • modal shell --cmd now can be shortened to modal shell -c. This means you can use it like modal shell -c "uname -a" to quickly run a command within the remote environment.

@ekzhang ekzhang requested a review from freider October 9, 2024 10:42
@mwaskom
Copy link
Contributor

mwaskom commented Oct 9, 2024

I was chatting with someone yesterday and they mentioned confusion between modal shell and modal container exec. That made me me wonder whether modal shell --container=<container_id> would be more intuitive than modal container exec <container_id> /bin/bash for getting a shell in a running container. In which case you might want -c to be the short form of --container, which is a long word.

On the other hand, bash -c etc. is pretty common, so it probably makes sense to have this too. Just mentioning!

@ekzhang
Copy link
Member Author

ekzhang commented Oct 9, 2024

Makes sense, I could look at adding that! How about modal shell ta-123456 directly then, replacing FUNC_REF?

@mwaskom
Copy link
Contributor

mwaskom commented Oct 9, 2024

That could also work! Really just a non-blocking comment either way, was just top of mind.

@ekzhang
Copy link
Member Author

ekzhang commented Oct 14, 2024

@prbot approve

Copy link

@modal-pr-review-automation modal-pr-review-automation bot left a comment

Choose a reason for hiding this comment

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

Approved 👍. @freider will follow-up review this.

@ekzhang ekzhang merged commit b243b99 into main Oct 14, 2024
25 checks passed
@ekzhang ekzhang deleted the ekzhang/modal-shell-c branch October 14, 2024 17:38
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.

2 participants