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

Fixed workflow store pathing issue #2792 #2797

Merged
merged 6 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions docs/server.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ To try out TorchServe serving now, you can load the custom MNIST model, with thi
After this deep dive, you might also be interested in:
* [Logging](logging.md): logging options that are available

* [Metrics](metrics.md): details on metrics collection
* [Metrics](metrics.md): details on metrics collection

* [REST API Description](rest_api.md): more detail about the server's endpoints

Expand Down Expand Up @@ -68,7 +68,7 @@ optional arguments:
MODEL_STORE.
--log-config LOG_CONFIG
Log4j configuration file for TorchServe
--ncs, --no-config-snapshots
--ncs, --no-config-snapshots
Disable snapshot feature
--workflow-store WORKFLOW_STORE
Workflow store location where workflow can be loaded. Defaults to model-store
Expand Down Expand Up @@ -107,6 +107,14 @@ There are no default required arguments to start the server
1. **start**: optional, A more descriptive way to start the server.
1. **stop**: optional, Stop the server if it is already running.

#### Argument Priority:
Arguments can be set in multiple locations (ex: command line, config.properties). The following is the priority:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Arguments can be set in multiple locations (ex: command line, config.properties). The following is the priority:
Arguments can be set in multiple locations (ex: command line, config.properties). The following is the priority from highest to lowest:

Also, should we include where environment variable configurations stand in this list?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Environment variables take highest precedence but enable_envvars_config has to be set true in config.properties, so it isnt enabled by default. I can add it to the doc if needed.

1. Command line
2. Config properties
3. Default settings

Example: Setting `model-store` in config.properties and through command line will result in the the command line location being used and overriding the config.properties.

## Advanced Features

### Custom Services
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,8 @@ private ConfigManager(Arguments args) throws IOException {
String workflowStore = args.getWorkflowStore();
if (workflowStore != null) {
prop.setProperty(TS_WORKFLOW_STORE, workflowStore);
} else if (prop.getProperty(TS_WORKFLOW_STORE) == null) {
prop.setProperty(TS_WORKFLOW_STORE, prop.getProperty(TS_MODEL_STORE));
}

String[] models = args.getModels();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,32 @@ public void testNoEnvVars() throws ReflectiveOperationException, IOException {
Assert.assertEquals(4, configManager.getJsonIntValue("noop", "1.0", "batchSize", 1));
Assert.assertEquals(4, configManager.getJsonIntValue("vgg16", "1.0", "maxWorkers", 1));
}

@Test
public void testWorkflowState() throws ReflectiveOperationException, IOException {
System.setProperty("tsConfigFile", "src/test/resources/config_test_env.properties");
ConfigManager.Arguments args = new ConfigManager.Arguments();
args.setModels(new String[] {"noop_v0.1"});
args.setSnapshotDisabled(true);
ConfigManager.init(args);
ConfigManager configManager = ConfigManager.getInstance();
String workingDir = configManager.getModelServerHome();
Assert.assertEquals(
workingDir + "/frontend/archive/src/test/resources/workflows",
configManager.getWorkflowStore());
}

@Test
public void testNoWorkflowState() throws ReflectiveOperationException, IOException {
System.setProperty("tsConfigFile", "src/test/resources/config_test_workflow.properties");
ConfigManager.Arguments args = new ConfigManager.Arguments();
args.setModels(new String[] {"noop_v0.1"});
args.setSnapshotDisabled(true);
ConfigManager.init(args);
ConfigManager configManager = ConfigManager.getInstance();
String workingDir = configManager.getModelServerHome();
Assert.assertEquals(
workingDir + "/frontend/archive/src/test/resources/models",
configManager.getWorkflowStore());
}
}
55 changes: 55 additions & 0 deletions frontend/server/src/test/resources/config_test_workflow.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# debug=true
# vmargs=-Xmx128m -XX:-UseLargePages -XX:+UseG1GC -XX:MaxMetaspaceSize=32M -XX:MaxDirectMemorySize=10m -XX:+ExitOnOutOfMemoryError
inference_address=https://127.0.0.1:8443
management_address=unix:/tmp/management.sock
metrics_address=https://127.0.0.1:8445
# model_server_home=../..
model_store=../archive/src/test/resources/models
# workflow_store=../archive/src/test/resources/workflows
load_models=noop-v0.1,noop-v1.0
# number_of_netty_threads=0
# netty_client_threads=0
# default_workers_per_model=0
# job_queue_size=100
async_logging=true
default_response_timeout=120
unregister_model_timeout=120
# number_of_gpu=1
# cors_allowed_origin
# cors_allowed_methods
# cors_allowed_headers
# keystore=src/test/resources/keystore.p12
# keystore_pass=changeit
# keystore_type=PKCS12
private_key_file=src/test/resources/key.pem
certificate_file=src/test/resources/certs.pem
# max_response_size=6553500
max_request_size=10485760
# blacklist_env_vars=.*USERNAME.*|.*PASSWORD.*
# enable_envvars_config=false
# decode_input_request=true
models={\
"noop": {\
"1.0": {\
"defaultVersion": true,\
"marName": "noop.mar",\
"minWorkers": 1,\
"maxWorkers": 1,\
"batchSize": 4,\
"maxBatchDelay": 100,\
"responseTimeout": 120\
}\
},\
"vgg16": {\
"1.0": {\
"defaultVersion": true,\
"marName": "vgg16.mar",\
"minWorkers": 1,\
"maxWorkers": 4,\
"batchSize": 8,\
"maxBatchDelay": 100,\
"responseTimeout": 120\
}\
}\
}
metrics_config=src/test/resources/metrics_default.yaml
7 changes: 2 additions & 5 deletions ts/model_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import tempfile
from builtins import str
from typing import Dict

import psutil

from ts.arg_parser import ArgParser
Expand Down Expand Up @@ -104,8 +105,7 @@ def start() -> None:
sys.exit(1)
ts_conf_file = ts_config

platform_path_separator = {
"Windows": "", "Darwin": ".:", "Linux": ".:"}
platform_path_separator = {"Windows": "", "Darwin": ".:", "Linux": ".:"}
class_path = "{}{}".format(
platform_path_separator[platform.system()],
os.path.join(ts_home, "ts", "frontend", "*"),
Expand Down Expand Up @@ -177,9 +177,6 @@ def start() -> None:

cmd.append("-w")
cmd.append(args.workflow_store)
else:
cmd.append("-w")
cmd.append(args.model_store)

if args.no_config_snapshots:
cmd.append("-ncs")
Expand Down