-
Notifications
You must be signed in to change notification settings - Fork 4
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
Compatible with (but not mandatory) Java 21 #293
Conversation
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Quality Gate passedIssues Measures |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #293 +/- ##
=============================================
- Coverage 70.29% 70.27% -0.02%
- Complexity 1060 1061 +1
=============================================
Files 48 48
Lines 6073 6073
Branches 801 801
=============================================
- Hits 4269 4268 -1
- Misses 1457 1458 +1
Partials 347 347
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
To write up my findings before the weekend
|
@@ -10,7 +10,7 @@ doc: "Mutect 1.1.5" | |||
|
|||
hints: | |||
- class: DockerRequirement | |||
dockerPull: quay.io/jeltje/mutect |
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.
Some Docker repos are forked, some can be downloaded and reuploaded in place (by simply pulling and pushing with an older client, which I had handy on my laptop)
This is an example of forking, the pull/push updates don't leave a trace.
@@ -14,7 +14,15 @@ fi | |||
if [ "${TESTING_PROFILE}" = "toil-integration-tests" ]; then | |||
pip3 install --user toil[cwl]==7.0.0 | |||
else | |||
pip3 install --user -r https://raw.githubusercontent.com/dockstore/dockstore/develop/dockstore-webservice/src/main/resources/requirements/1.15.0/requirements3.txt | |||
sudo apt-get update | |||
# https://stackoverflow.com/questions/44331836/apt-get-install-tzdata-noninteractive needed by cwltool |
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.
This matches 1.16 quickstart with pipx, but had to do a bit of extra work to get schema_salad and cwlref-runner (may be using pipx runpip wrong)
This reverts commit fa84e68.
Quality Gate passedIssues Measures |
@@ -17,14 +17,14 @@ executors: | |||
|
|||
machine_integration_test_exec: | |||
machine: # run the steps with Ubuntu VM | |||
image: ubuntu-2204:2023.10.1 | |||
image: ubuntu-2204:2024.04.4 |
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.
Why is the tag different for this and the toil_wes_test_executor
below?
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.
A sad reason
While experimenting, I bumped to a newer version to try to get a newer Java version on the machine executor. It turned out to be unnecessary, but since we ran the tests anyway I kept the newer versions that worked (toil_wes_test_executor
used for wes-toil-test
).
Something about ubuntu-2204:2024.11.1
breaks the toil install silently (but not all of toil, the part used for regular integration tests including integration-tests-toil-integration-tests
, not a typo)
Description
Make the CLI build compatible with Java 21.
Does not switch to only Java 21, yet
Some notes:
dockstore-testing
, in others just download and uploading with a new daemonquay.io/dockstore-testing
, some workflows modified in testing repos such as https://github.com/dockstore-testing/dockstore-tool-md5sum https://github.com/dockstore-testing/mutect-tool https://github.com/dockstore-testing/dockstore-tool-linux-sort but in the end, I mostly downloaded and uploaded Docker images using a compatible Docker daemon rather than fixing old DockerfilesReview Instructions
Could do some spot checking, listing and running workflows as in our getting started tutorials
Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-6611
Follow-up with https://ucsc-cgl.atlassian.net/browse/SEAB-6851
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!
./mvnw clean install