-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
Experimental Docker Support #3781
Experimental Docker Support #3781
Conversation
Issue: |
Have you merged in the fix around that? |
I propose we merge this without system test support inside of docker (you can still run them on the host), and follow-up with that. I've made progress on the system tests, but I don't want to hold this up or make it even more complicated than it already is. There is one test failing on this PR because it was opened from a fork and knapsack is refusing to run. I can close and reopen if preferred. |
Right, for clarity -- the system specs don't work in docker locally. This shouldn't affect anything outside of local-docker-usage. (above fails are likely flaky specs :( ) |
System tests are running in docker now! I'd appreciate one or two people testing this out on their machines, especially if you're on Linux. |
Finally fixed the organization spec that was breaking due to the factory changes to remove the ActiveStorage error. Also, request specs were not working, because they were all running at example.com locally which was not in the allowed hosts in the test configuration. |
I created an issue to tie together this work and a few others, #3829 |
As mentioned in 71790db, there's an ActiveStorage error that creeps up in Docker. Using attach with the Factory prevents this error. Transient property is used so we can override the attachment in specific tests.
02eea03
to
b956fa4
Compare
@awwaiid I'm not sure I'm onboard with the approach here. If we're explicitly not supporting Docker, I worry that having any reference to it in the codebase is just going to confuse people. The changes here will need to be maintained no matter what (e.g. references to Docker-specific hostnames in config files). I personally have never had success with running a Rails app in Docker - I found it a lot better to run Rails locally but have its dependencies (Redis, Postgres) in Docker. It's a bit of a pain to set up, but it's so much simpler to work with the actual code once it is. I think I'd rather just add a comment to the README indicating that Docker is not supported, and close this. Thoughts? |
@dorner I use docker all the time, and some of this was based on the local files I have that aren't committed. This PR hasn't been updated since the switch to cuprite, which I think obsoletes several of the spec changes, and might bring this down in size even further. Still, there is a gap between "Works For Me" and "Do It This Way" which we haven't quite achieved. I have a dream over in #3829 that we can get a one-click codespace-powered environment set up, and that requires almost exactly what this requires. So how about this -- I'm closing this PR as the most recent attempt that didn't quite get to merge-maintainable. I'll keep my eye on the codespace dream and see if I can make that happen. It generally is the same as local-docker, with the compose file living in a subdirectory, albeit likely with some of of those carefully-targeted (and hopefully generic (Thank you for your work on this @ChaelCodes, it will help me with the CodeSpaces setup and we'll get it next time!) |
Have you tried Nix before? It kinda similar to Docker, but more local build script based vs container based. I've been working on a PR to add that as a setup option. #4185 |
Description
Docker has a long and storied history in human-essentials. Many contributors (and maintainers!) want to use it, but it's challenging to maintain and has frequently resulted in issues. Instead of supporting Docker, I think it'd be good to balance and compromise between these two needs by adding Docker, but providing it as an experimental example, instead of supporting it fully.
**Dockerfile and docker-compose.yml provided by @awwaiid **
Type of change
How Has This Been Tested?
This Docker setup has been run locally by @ChaelCodes and @awwaiid
The entire test suite was run locally by @ChaelCodes .