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

Adds configuration for local file system connector. #25

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

avaz
Copy link

@avaz avaz commented Jul 18, 2024

Adds Local File System volume configuration with environment variable support.

avaz added 2 commits July 18, 2024 17:40
Adds Local File System volume configuration with environment variable support.
@avaz avaz changed the title Update docker-compose.yaml Adds configuration for local file system connector. Jul 18, 2024
@@ -24,6 +24,9 @@ RAY_MEMORY=100G
FRONTEND_PROTOCOL=http
SECURED_COOKIES="False"

# Path to the local host file system where the Local File System connector will write the data
SYNTHO_LOCALFS=/tmp/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add this env in this .tpl file here? syntho-cli is using this .tpl when generating the real .env file when deploying. We actually have a PR checks to avoid this discrepancy but apparently we missed adding this check for docker compose, and added for helm charts only. We talked with @Younday and that PR check will be added soon for docker compose as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's maybe also a good thing to check whether we want to merge this in, I believe it was a temporary fix. I will however create another PR with a check in a pipeline to see if the keys in .env.tpl and example.env.

@Younday Younday added the on hold label Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants