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

Improve docker image UX and fix MNIST serving example #3382

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jurca
Copy link

@jurca jurca commented Jan 22, 2025

Description

Improve ease of use for the docker image and fix the MNIST serving example.

I haven't filed an issue, and this does not seem to be reported (or I missed it - sorry!).

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Feature/Issue validation/testing

I've tested the changes only manually, as they are very small.

Checklist:

  • Did you have fun?
    • Sorry, no, it has been frustrating finding out what's wrong and how to fix it - but that's why I created this PR, so that others and future me will instead have fun.
  • Have you added tests that prove your fix is effective or that this feature works?
    • I think it might not be necessary in this case.
  • Has code been commented, particularly in hard-to-understand areas?
  • Have you made corresponding changes to the documentation?

jurca added 4 commits January 22, 2025 14:31
This removes the need to run tail to prevent docker exit, and ensures
that signals will be passed by shell to torchserve, allowing it to
terminate gracefully.

This change moves the tail call to be used only if not running in serve
(default) mode, allowing docker exit, so that the container can be
restarted by the orchestrator, instead of leaving a container that no
longer runs a torchserve instance (this improves UX with Kubernetes).
This improves usability of out-of-box TorchServe docker image without
having to create a custom image or set configuration as command line
options.
The model API is disabled by default, so it must be enabled first, or
the model needs to be registered in an alternative way.
@jurca
Copy link
Author

jurca commented Jan 23, 2025

Additionally, if the tail -f /dev/null in docker/dockerd-entrypoint.sh was present only to prevent container termination when torchserve was running as daemon in background, I would like to propose removing it completely, as I find it detrimental to UX when using custom command.

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