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

Make readiness_endpoint liveness_endpoint required to use custom server #1267

Merged
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
8 changes: 4 additions & 4 deletions truss/base/truss_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,17 +421,17 @@ class DockerServer:
start_command: str
server_port: int
predict_endpoint: str
readiness_endpoint: Optional[str] = None
liveness_endpoint: Optional[str] = None
readiness_endpoint: str
liveness_endpoint: str

@staticmethod
def from_dict(d) -> "DockerServer":
return DockerServer(
start_command=d.get("start_command"),
server_port=d.get("server_port"),
predict_endpoint=d.get("predict_endpoint"),
readiness_endpoint=d.get("readiness_endpoint", None),
liveness_endpoint=d.get("liveness_endpoint", None),
readiness_endpoint=d.get("readiness_endpoint"),
liveness_endpoint=d.get("liveness_endpoint"),
)

def to_dict(self):
Expand Down
6 changes: 6 additions & 0 deletions truss/contexts/image_builder/serving_image_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,12 @@ def generate_docker_server_nginx_config(build_dir, config):
assert (
config.docker_server.server_port is not None
), "docker_server.server_port is required to use custom server"
assert (
config.docker_server.readiness_endpoint is not None
), "docker_server.readiness_endpoint is required to use custom server"
assert (
config.docker_server.liveness_endpoint is not None
), "docker_server.liveness_endpoint is required to use custom server"
Copy link
Contributor

Choose a reason for hiding this comment

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

These should probably be exepctions...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Assertions are primarily used as a debugging aid. They are meant to catch programming errors during development by verifying assumptions made by the programmer.

Exceptions are used for handling errors and other "exceptional" conditions that may arise at runtime due to external factors (e.g., invalid user input, file I/O, network errors).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it will change to exceptions in the next pr


nginx_content = nginx_template.render(
server_endpoint=config.docker_server.predict_endpoint,
Expand Down
4 changes: 0 additions & 4 deletions truss/templates/docker_server/proxy.conf.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ server {
listen 8080;
client_max_body_size {{client_max_body_size}};

{%- if liveness_endpoint %}
# Liveness endpoint override
location = / {
proxy_redirect off;
Expand All @@ -13,8 +12,6 @@ server {

proxy_pass http://127.0.0.1:{{server_port}};
}
{%- endif %}
{%- if readiness_endpoint %}
# Readiness endpoint override
location ~ ^/v1/models/model$ {
proxy_redirect off;
Expand All @@ -24,7 +21,6 @@ server {

proxy_pass http://127.0.0.1:{{server_port}};
}
{%- endif %}
# Predict
location ~ ^/v1/models/model:predict$ {
proxy_redirect off;
Expand Down
2 changes: 2 additions & 0 deletions truss/tests/test_data/test_custom_server_truss/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ docker_server:
start_command: python main.py
predict_endpoint: /predict
server_port: 8000
readiness_endpoint: /
liveness_endpoint: /
resources:
accelerator: null
cpu: '1'
Expand Down