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

Docker rules engine issue 116 #194

Conversation

ethanstrominger
Copy link
Collaborator

@ethanstrominger ethanstrominger commented May 31, 2024

See issue #116

Docker will only process sub directories so it needed to be above rules-engine and heat-stack directory.

I created a copy of the original Dockerfile to /Dockerfile-original to make comparison easier.

@ethanstrominger ethanstrominger self-assigned this May 31, 2024
@ethanstrominger ethanstrominger requested a review from thadk May 31, 2024 21:19
@thadk
Copy link
Member

thadk commented May 31, 2024

Hi @ethanstrominger thanks so much for this!

I just want to note that though we didn't get to check the final insertion-into-docker step, hypothetically in this PR from 10 days ago #176 gets the rules-engine built python egg into the GitHub Action (GHA) artifacts. Then the vitest uses it from the GHA artifacts. I figured we could get that into docker as it was building.

Which approach or mix of approaches do you think we should ultimately use? Thankfully, with both these "new" ways, whatever rules-engine branch version should exactly match the deployed one which was our worry with our first other discarded solution to this.

Sorry I didn't get back to update that original issue with the progress 🫤!

We can totally merge in the docker way: my only tiny reservation is on extra divergence from our docker deploy template which may yet get new features upstream.
There is not any automated upgrade process though so just a bit of extra hand merging later. Also we might need to double check the .dockerignore changes.

@ethanstrominger
Copy link
Collaborator Author

Thad - Whatever you think is best. I don't know enough about the other way to judge. With the docker approach I can test without checking into github.

@thadk
Copy link
Member

thadk commented Jun 1, 2024

That's a great point about being able to test locally! Let's use this.

Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@thadk thadk left a comment

Choose a reason for hiding this comment

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

Please test on this repo's dev branch (force push is fine if needed, we're not currently using gitflow or anything like that) and get GHA deploy stage working to heat-stack-staging.fly.dev .

Particularly you'll prolly have to adjust these paths:

- name: 🚚 Move Dockerfile
run: |
mv ./other/Dockerfile ./Dockerfile
mv ./other/.dockerignore ./.dockerignore

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.

3 participants