From 5388162e69f29a6591557e05e5ea39b8d37e4a2f Mon Sep 17 00:00:00 2001 From: Justin Variath Thomas Date: Wed, 2 Aug 2023 11:14:53 -0400 Subject: [PATCH] WX-1192 Updated semver versions for flagged dependencies (#790) * WX-1192 updated semver versions for flagged dependencies * WX-1192 added constraints.txt so that it doesn't install and break on cython 3 or above, updated tox.ini and swagger-codegen-ignore * WX-1192 updated dockerfiles and readme to include constraints.txt * WX-1192 added oxford comma * WX-1192 updated pip install commands to correctly account for constraints when installing PyYAML --- README.md | 2 +- servers/cromwell/.swagger-codegen-ignore | 1 + servers/cromwell/Dockerfile | 10 ++++++++-- servers/cromwell/Dockerfile.dev | 10 +++++++++- servers/cromwell/constraints.txt | 1 + servers/cromwell/requirements.txt | 9 ++++++++- servers/cromwell/tox.ini | 1 + ui/package.json | 4 ++++ ui/yarn.lock | 11 +++++++++++ 9 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 servers/cromwell/constraints.txt diff --git a/README.md b/README.md index 77a75a1f0..0714ff222 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ Monitors jobs launched by the [Cromwell workflow engine](https://github.com/broa #### Notes 1. Websocket reload on code change does not work in docker-compose (see https://github.com/angular/angular-cli/issues/6349). -2. Changes to `package.json` or `requirements.txt` or [regenerating the API](#updating-the-api-using-swagger-codegen) require a rebuild with: +2. Changes to `package.json`, `requirements.txt`, or `constraints.txt` [regenerating the API](#updating-the-api-using-swagger-codegen) require a rebuild with: ``` docker-compose up --build ``` diff --git a/servers/cromwell/.swagger-codegen-ignore b/servers/cromwell/.swagger-codegen-ignore index 216f86861..a672b11dc 100644 --- a/servers/cromwell/.swagger-codegen-ignore +++ b/servers/cromwell/.swagger-codegen-ignore @@ -7,6 +7,7 @@ README.md .gitignore tox.ini test-requirements.txt +constraints.txt # These have been hand-modified, no need to regenerate as they would get overwritten. requirements.txt diff --git a/servers/cromwell/Dockerfile b/servers/cromwell/Dockerfile index 36349deb3..5e93df0fa 100644 --- a/servers/cromwell/Dockerfile +++ b/servers/cromwell/Dockerfile @@ -13,8 +13,14 @@ WORKDIR /app COPY --from=0 /job-manager/servers/jm_utils /app/jm_utils COPY --from=0 /job-manager/servers/cromwell/jobs /app/jobs COPY ./servers/cromwell/ /app/jobs - -RUN cd jobs && pip3 install -r requirements.txt +# Below is a link explaining where the individual PyYAML install command comes from +# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 +# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it will fail to install due to deprecated features +# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package +# This is problematic for requirements.txt as it cannot specify a wheel, only a source package +# So we need to install PyYAML separately with the constraint defined in constraints.txt +RUN cd jobs && PIP_CONSTRAINT=constraints.txt pip install PyYAML==5.4.1 +RUN cd jobs && pip install -r requirements.txt # We installed jm_utils so don't need local copy anymore, which breaks imports RUN rm -rf jm_utils diff --git a/servers/cromwell/Dockerfile.dev b/servers/cromwell/Dockerfile.dev index 0df2403f0..42a19f8d2 100644 --- a/servers/cromwell/Dockerfile.dev +++ b/servers/cromwell/Dockerfile.dev @@ -7,7 +7,15 @@ WORKDIR /app ADD servers/jm_utils /app/jm_utils ADD servers/cromwell/jobs /app/jobs COPY servers/cromwell/requirements.txt /app/jobs -RUN cd jobs && pip3 install -r requirements.txt +COPY servers/cromwell/constraints.txt /app/jobs +# Below is a link explaining where the individual PyYAML install command comes from +# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 +# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it will fail to install due to deprecated features +# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package +# This is problematic for requirements.txt as it cannot specify a wheel, only a source package +# So we need to install PyYAML separately with the constraint defined in constraints.txt +RUN cd jobs && PIP_CONSTRAINT=constraints.txt pip install PyYAML==5.4.1 +RUN cd jobs && pip install -r requirements.txt # We installed jm_utils so don't need local copy anymore, which breaks imports RUN rm -rf jm_utils diff --git a/servers/cromwell/constraints.txt b/servers/cromwell/constraints.txt new file mode 100644 index 000000000..f21f1571c --- /dev/null +++ b/servers/cromwell/constraints.txt @@ -0,0 +1 @@ +cython<3.0.0 \ No newline at end of file diff --git a/servers/cromwell/requirements.txt b/servers/cromwell/requirements.txt index a7ff0d02e..4fdd71194 100644 --- a/servers/cromwell/requirements.txt +++ b/servers/cromwell/requirements.txt @@ -17,7 +17,14 @@ MarkupSafe==2.1.1 pathlib==1.0.1 python-dateutil==2.6.0 pytz==2022.4 -PyYAML==5.4 +# Below is a link explaining why the PyYAML package is commented out +# https://github.com/yaml/pyyaml/issues/736#issuecomment-1653209769 +# In short, due to Cython 3 being released, PyYAML needs to have it's Cython dependency contrained, otherwise it'll fail to install due to deprecated features +# However installation of PyYAML uses a "wheel", which is basically a pre-compiled version of the package +# This is problematic for requirements.txt as it cannot specify a wheel, only a source package +# So we need to install PyYAML separately with the constraint defined in constraints.txt via pip install as opposed to here +# You can see the above implemented in the Dockerfiles +# PyYAML==5.4 requests==2.28.1 six==1.11.0 swagger-spec-validator==2.7.6 diff --git a/servers/cromwell/tox.ini b/servers/cromwell/tox.ini index fd606fa9b..645784d35 100644 --- a/servers/cromwell/tox.ini +++ b/servers/cromwell/tox.ini @@ -2,6 +2,7 @@ envlist = py310 [testenv] +setenv=PIP_CONSTRAINT={toxinidir}/constraints.txt deps=-r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt diff --git a/ui/package.json b/ui/package.json index 0be7c4874..8fdc28e51 100644 --- a/ui/package.json +++ b/ui/package.json @@ -63,5 +63,9 @@ "ts-node": "^10.9.1", "typescript": "~4.8.2" }, + "resolutions": { + "node-gyp/semver@^7.3.5": "7.5.2", + "@npmcli/fs/semver@^7.3.5": "7.5.2" + }, "packageManager": "yarn@3.2.2" } diff --git a/ui/yarn.lock b/ui/yarn.lock index ca6291321..c6846e50a 100644 --- a/ui/yarn.lock +++ b/ui/yarn.lock @@ -9250,6 +9250,17 @@ __metadata: languageName: node linkType: hard +"semver@npm:7.5.2": + version: 7.5.2 + resolution: "semver@npm:7.5.2" + dependencies: + lru-cache: ^6.0.0 + bin: + semver: bin/semver.js + checksum: 3fdf5d1e6f170fe8bcc41669e31787649af91af7f54f05c71d0865bb7aa27e8b92f68b3e6b582483e2c1c648008bc84249d2cd86301771fe5cbf7621d1fe5375 + languageName: node + linkType: hard + "semver@npm:^5.3.0, semver@npm:^5.6.0": version: 5.7.1 resolution: "semver@npm:5.7.1"