Skip to content

Commit

Permalink
WX-1192 Updated semver versions for flagged dependencies (#790)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
JVThomas authored Aug 2, 2023
1 parent 035a5fe commit 5388162
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 5 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
```
Expand Down
1 change: 1 addition & 0 deletions servers/cromwell/.swagger-codegen-ignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions servers/cromwell/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 9 additions & 1 deletion servers/cromwell/Dockerfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions servers/cromwell/constraints.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
cython<3.0.0
9 changes: 8 additions & 1 deletion servers/cromwell/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions servers/cromwell/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
envlist = py310

[testenv]
setenv=PIP_CONSTRAINT={toxinidir}/constraints.txt
deps=-r{toxinidir}/requirements.txt
-r{toxinidir}/test-requirements.txt

Expand Down
4 changes: 4 additions & 0 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
11 changes: 11 additions & 0 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 5388162

Please sign in to comment.