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

TorchServe startup fails to honor valid combinations of config.properties and command line #63

Closed
fbbradheintz opened this issue Feb 21, 2020 · 3 comments
Labels
bug Something isn't working launch blocker triaged_wait Waiting for the Reporter's resp

Comments

@fbbradheintz
Copy link
Contributor

I have models in my model_store folder, and the line model_store=model_store in my config.properties file.

The following two command lines work, with the first starting no endpoints and the second starting workers for the model:

torchserve --start --ts-config config_with_batch.properties
torchserve --start --ts-config config_with_batch.properties --models d161good=d161good.mar  --model-store model_store

In the first one, it takes the model store location from config; in the second, it ignores that in favor of the command line.

This command line fails (error message in the second line):

> torchserve --start --ts-config config_with_batch.properties --models d161good=d161good.mar
--model-store is required to load model locally.

Again the model store is specified, just not on the command line. TorchServe should check the config for missing parameters before rejecting a command line.

@mycpuorg
Copy link
Collaborator

Got it. Fallback to config properties when command line specifies incorrect properties.

IOW, this can be regarded as a "user override config failure". As in, user tried to override the config properties and passed incorrect configs. I'd argue that this is not a launch blocker, in fact, I would argue this is not a bug but requires a better error message.

@mycpuorg mycpuorg added enhancement New feature or request incorrect_error_msg and removed bug Something isn't working launch blocker enhancement New feature or request labels Feb 25, 2020
@harshbafna harshbafna added launch blocker triaged_wait Waiting for the Reporter's resp bug Something isn't working and removed incorrect_error_msg labels Feb 26, 2020
@harshbafna
Copy link
Contributor

#68 should fix this.

@fbbradheintz
Copy link
Contributor Author

fbbradheintz commented Feb 26, 2020

With the line model_store=model_store in the config file, and my model archive in the model_store folder, all of the following work:

torchserve --start --ts-config config.properties
torchserve --start --ts-config config.properties --model-store model_store
torchserve --start --ts-config config.properties --model-store model_store --models d161good=d161good.mar
torchserve --start --ts-config config.properties --models d161good=d161good.mar

With the same config and my model in the folder model_stash, the following work:

torchserve --start --ts-config config.properties
torchserve --start --ts-config config.properties --model-store model_stash
torchserve --start --ts-config config.properties --model-store model_stash --models d161good=d161good.mar

Under those conditions, the following starts the server but fails to start workers (which is expected, because it is looking in the wrong folder for the model):

torchserve --start --ts-config config.properties --models d161good=d161good.mar

This looks good - I'm merging. (I verified that the branch rebases onto master cleanly.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working launch blocker triaged_wait Waiting for the Reporter's resp
Projects
None yet
Development

No branches or pull requests

3 participants