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

Fix rclone path in Docker image #125

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

Fix rclone path in Docker image #125

wants to merge 4 commits into from

Conversation

webb-ben
Copy link
Member

@webb-ben webb-ben commented Mar 6, 2025

Fix rclone path in docker image by installing during Docker build
Addresses: #122

@webb-ben webb-ben requested a review from C-Loftus March 6, 2025 14:05
@C-Loftus
Copy link
Member

C-Loftus commented Mar 6, 2025

I'm thinking, might it just be easier to move the rclone binary to the PATH in the container? That way we don't have to install in docker. Since we have 4 different scenarios: production, pytest in ci/cd, pytest locally, or local dagster dev, I feel like it may start to make it confusing to have essentially dependencies that are installed differently. I think having rclone in the pipeline is ugly but keeps it consistent.

I think this is cleaner than using the static method, just trying to prevent drift between dev/prod

@webb-ben
Copy link
Member Author

webb-ben commented Mar 6, 2025

The docker container runs from root. I do not think it is a safe assumption that the behavior inside the container is not something we will consistently see outside of the container. Leave the pipeline as is, and just install Docker appropriately using in the Dockerfle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants