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
@uppy/companion: fix Dockerfile and deploy automation #3355
Conversation
that works with yarn workspaces and respects yarn.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd need to change those lines as well
uppy/.github/workflows/companion-deploy.yml
Lines 35 to 36 in 222b873
context: packages/@uppy/companion | |
file: packages/@uppy/companion/Dockerfile |
It's a bit unfortunate that the Docker files for Companion is no longer in the @uppy/companion
folder, but if that's all it needs, that's an acceptable tradeoff.
Dockerfile
Outdated
|
||
RUN apk --update add --virtual native-dep \ | ||
make gcc g++ python3 libgcc libstdc++ git && \ | ||
cd /app && corepack yarn plugin import workspace-tools && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workspace-tools
plugin should be already in /app/.yarn
, can we remove this instruction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops i missed that,thanks!
Yea, it might be possible to make it work with Dockerfile inside packages/@uppy/companion too, but I'm not enough of a docker guru to fix that. It would have to include files from parent directories and also work with docker-compose.yml. |
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Are you moving the Dockerfile outside of companion package folder? Why? 😅 |
I couldn't find a way to run yarn install inside docker from the root of the monorepo (where yarn.lock resides) without setting the docker working directory to the root of the monorepo. if you know how, then please let me know :) can we simply put |
So the Docker build job completed successfully, but the Heroku one failed: https://github.com/transloadit/uppy/runs/4473760891?check_suite_focus=true... |
| Package | Version | Package | Version | | ----------------- | ------- | ----------------- | ------- | | @uppy/angular | 0.2.7 | @uppy/store-redux | 2.0.3 | | @uppy/audio | 0.2.1 | @uppy/svelte | 1.0.6 | | @uppy/aws-s3 | 2.0.7 | @uppy/vue | 0.4.4 | | @uppy/companion | 3.1.3 | @uppy/xhr-upload | 2.0.7 | | @uppy/core | 2.1.4 | @uppy/robodog | 2.1.5 | | @uppy/dashboard | 2.1.3 | uppy | 2.3.1 | | @uppy/locales | 2.0.5 | | | - meta: update npm deps (Antoine du Hamel / #3352) - @uppy/companion: fix Dockerfile and deploy automation (Mikael Finstad / #3355) - @uppy/companion: don’t pin Yarn version in `package.json` (Antoine du Hamel / #3347) - @uppy/aws-s3,@uppy/core,@uppy/dashboard,@uppy/store-redux,@uppy/xhr-upload: deps: use `nanoid/non-secure` to workaround react-native limitation (Antoine du Hamel / #3350) - @uppy/audio: showRecordingLength option was removed, always clearInterval (Artur Paikin / #3351) - meta: drop `stringify-object` dependency to generate locales (Antoine du Hamel / #3344) - meta: add release automations (Antoine du Hamel / #3304)
* attempt at fixing dockerfile that works with yarn workspaces and respects yarn.lock * run corepack yarn * update Dockerfile references * remove unneccesary yarn plugin import * Apply suggestions from code review Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
| Package | Version | Package | Version | | ----------------- | ------- | ----------------- | ------- | | @uppy/angular | 0.2.7 | @uppy/store-redux | 2.0.3 | | @uppy/audio | 0.2.1 | @uppy/svelte | 1.0.6 | | @uppy/aws-s3 | 2.0.7 | @uppy/vue | 0.4.4 | | @uppy/companion | 3.1.3 | @uppy/xhr-upload | 2.0.7 | | @uppy/core | 2.1.4 | @uppy/robodog | 2.1.5 | | @uppy/dashboard | 2.1.3 | uppy | 2.3.1 | | @uppy/locales | 2.0.5 | | | - meta: update npm deps (Antoine du Hamel / transloadit#3352) - @uppy/companion: fix Dockerfile and deploy automation (Mikael Finstad / transloadit#3355) - @uppy/companion: don’t pin Yarn version in `package.json` (Antoine du Hamel / transloadit#3347) - @uppy/aws-s3,@uppy/core,@uppy/dashboard,@uppy/store-redux,@uppy/xhr-upload: deps: use `nanoid/non-secure` to workaround react-native limitation (Antoine du Hamel / transloadit#3350) - @uppy/audio: showRecordingLength option was removed, always clearInterval (Artur Paikin / transloadit#3351) - meta: drop `stringify-object` dependency to generate locales (Antoine du Hamel / transloadit#3344) - meta: add release automations (Antoine du Hamel / transloadit#3304)
Because I realised that we're not using PnP, I attempted fixing dockerfile so that it works with yarn workspaces and respects yarn.lock
Alternative to #3347
had to set hoisting limit for companion so that all node_modules are kept inside companion folder, I think this makes sense in general because companion is kind of on its own
i tested the docker file and it launches successfully