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

Locally use volumes for workers code #766

Merged
merged 5 commits into from
Feb 3, 2023

Conversation

lhoestq
Copy link
Member

@lhoestq lhoestq commented Feb 2, 2023

This way in a local dev env, one can restart a single worker container to apply their code changes instead of running make stop and make start again

@lhoestq lhoestq requested a review from severo February 2, 2023 15:05
Copy link
Collaborator

@severo severo left a comment

Choose a reason for hiding this comment

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

Good idea!

The CI is failing because the e2e tests use the docker compose, and it doesn't work for some reason with this new setup.

@lhoestq
Copy link
Member Author

lhoestq commented Feb 2, 2023

I ended up defining a separate docker-compose file for dev.

This way the e2e tests can use the actual docker image that we may deploy.

I introduced

make dev-start

@severo severo self-requested a review February 3, 2023 09:27
Copy link
Collaborator

@severo severo left a comment

Choose a reason for hiding this comment

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

OK, nice!

Two small comments:

@lhoestq
Copy link
Member Author

lhoestq commented Feb 3, 2023

Yea we'll just have to remember to update both when modifying something, but it should be fine since they're next to each other.

@lhoestq lhoestq merged commit 3bb80b7 into main Feb 3, 2023
@lhoestq lhoestq deleted the locally-use-volumes-for-workers-code branch February 3, 2023 11:09
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.

None yet

2 participants