Fix Docker build and runtime issues for cross-platform support #243
Conversation
Deploying geoinsight with
|
| Latest commit: |
56e8063
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://97f2b1fb.geoinsight.pages.dev |
| Branch Preview URL: | https://fix-docker-cross-platform-su.geoinsight.pages.dev |
annehaley
left a comment
annehaley
left a comment
There was a problem hiding this comment.
I was able to successfully build and run all containers on this branch, both with the gpu profile and without. I made some minor suggestions. I will also request review from @augustposch on this; he will be able to provide further insight as a windows user.
|
I just did a fresh |
augustposch
left a comment
augustposch
left a comment
There was a problem hiding this comment.
After fixing the instances of ./manage.py which I pointed out in my comments, this branch succeeded for me on Windows, up to the point of seeing the login screens. I'm looking forward to getting in the app once I know how to log in.
thanks for testing the PR @zachmullen |
aashish24
force-pushed
the
fix/docker-cross-platform-support
branch
5 times, most recently
from
e37125a to
c123905
Compare
Build fixes: - Upgrade rasterio to 1.3.11 (fixes numpy==2.0.0rc1 build dependency) - Add g++ and libgdal-dev for building rasterio from source on ARM - Set GDAL_CONFIG env var for Fiona/tile2net builds Runtime fixes: - Add optional GPU profile for NVIDIA systems (celery-gpu service) - Default celery runs CPU-only, use --profile gpu for GPU acceleration - Fix web container to install npm dependencies inside container - Use named volume for node_modules to avoid host/container arch mismatch - Update Node.js to v22 (required by Vite 7.x) - Remove platform: linux/amd64 from postgres for native ARM performance Documentation: - Update setup.md with GPU instructions and troubleshooting - Improve formatting and add prerequisites section
- Fix Windows compatibility: use 'python manage.py' instead of './manage.py' - Remove shm_size from default celery (only needed for GPU) - Update GPU celery comment for generic accelerated inferencing - Fix GPU command flag order (--profile before 'up', --scale after) - Fix ingest documentation paths (relative to sample_data/) - Add API Documentation (Swagger) URL to access points - Clarify createsuperuser creates login credentials
aashish24
force-pushed
the
fix/docker-cross-platform-support
branch
from
c123905 to
3da8e86
Compare
|
@annehaley @augustposch this PR is ready for your another review. |
- Remove npm install step (now handled inside container) - Remove Node.js/npm from prerequisites - Remove tile2net references (tile2net has been removed)
|
@annehaley thanks for your review. I addressed your comments. I kept reference to node installation in case developers might work on frontend outside containers |
augustposch
left a comment
augustposch
left a comment
There was a problem hiding this comment.
This time `docker compose up` didn't work for me due to missing semicolon. Skimming the yaml files, I'm not sure why there would be a problem with a semicolon in line 48. Any suggestion for me?
- Add winpty prefix note for createsuperuser command on Windows - Add Windows-Specific Issues troubleshooting section - Document fix for interactive commands hanging in Git Bash/MINGW - Document fix for 'No such container' errors
|
@augustposch and @annehaley I pushed changes to the document based on @augustposch's input. |
Build fixes:
Runtime fixes:
Documentation: