Update Dockercompose with more info #1699
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 54c35a7 in 7 seconds
More details
- Looked at
25lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. docker-compose.yml:39
- Draft comment:
Consider adding a comment explaining the purpose of theLLM_KEYenvironment variable and its possible values, similar to the comments provided for other LLM providers. - Reason this comment was not posted:
Confidence changes required:50%
The environment variable 'LLM_KEY' is set to 'OPENAI_GPT4O', but there is no explanation or comment about its purpose or usage. This might confuse users who are not familiar with it.
Workflow ID: wflow_xKg3sAIEOD0asg5R
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 16ceb3d in 11 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. docker-compose.yml:100
- Draft comment:
The comment "(maybe not needed)" is ambiguous and could lead to confusion. Consider providing more context or removing it if it's not necessary. - Reason this comment was not posted:
Confidence changes required:50%
The comment change is minor but could lead to confusion.
Workflow ID: wflow_596DfZFboflykJIT
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6a4e9f2 in 10 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. docker-compose.yml:104
- Draft comment:
The comment update provides useful information for obtaining the API key, which improves clarity for users setting up the environment. - Reason this comment was not posted:
Confidence changes required:0%
The change in the PR is a comment update to provide more information about obtaining the API key. This is a documentation improvement and does not affect the functionality of the code.
Workflow ID: wflow_vsag3b8X3CnP5bWk
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| # - VITE_API_BASE_URL=http://localhost:8000/api/v1 | ||
| # - VITE_SKYVERN_API_KEY= | ||
| # - VITE_SKYVERN_API_KEY=<get this from "settings" in the Skyvern UI> |
There was a problem hiding this comment.
skyvern/entrypoint-skyvernui.sh
Line 7 in 204972e
currently we use the api key in .streamlit/secrets.toml so this is not a required step here.
we should migrate to a better solution - use an .env file
There was a problem hiding this comment.
not high priority imo!
| # If you're self-hosting this behind a dns, you'll want to set: | ||
| # A route for the API: api.yourdomain.com -> localhost:8000 | ||
| # A route for the UI: yourdomain.com -> localhost:8080 | ||
| # A route for the artifact API: artifact.yourdomain.com -> localhost:9090 (maybe not needed) |
There was a problem hiding this comment.
@msalihaltun is the 9090 port still used? if not we should remove it.
There was a problem hiding this comment.
it's used for the artifacts endpoint
| @@ -93,9 +94,14 @@ services: | |||
| environment: | |||
| # if you want to run skyvern on a remote server, | |||
| # you need to change the host in VITE_WSS_BASE_URL and VITE_API_BASE_URL to match your server ip | |||
| # If you're self-hosting this behind a dns, you'll want to set: | |||
| # A route for the API: api.yourdomain.com -> localhost:8000 | |||
There was a problem hiding this comment.
this is a bit confusing to me. when I set up dns routing, I won't use localhost. what we want to say here is to point their api.yourdomain.com to the hosting service's dns or ip, port 8000
so VITE_API_BASE_URL should be https://api.yourdomain.com/api/v1, which gets routed to https://host_service:8000/api/v1
There was a problem hiding this comment.
that's correct, and I'm not sure if this commetn is that confusing. I'm OK to let our users say that because only our very very technical users are going to be reading this comment and leveraging it.
Important
Enhance
docker-compose.ymlwith newLLM_KEYand detailed DNS routing instructions for self-hosting.LLM_KEY=OPENAI_GPT4Otoskyvernservice.VITE_SKYVERN_API_KEYinskyvern-uiservice to guide users on obtaining the key.skyvern-uiservice.This description was created by
for 6a4e9f2. It will automatically update as commits are pushed.