change http back to https to make mlcroissant validate happy#12333
change http back to https to make mlcroissant validate happy#12333stevenwinship merged 1 commit intodevelopfrom
mlcroissant validate happy#12333Conversation
This commit reverts 9e619b6 from #12214. We were following the Croissant 1.1 spec but there's a mismatch between it and `mlcroissant validate`. We've opened this issue upstream: mlcommons/croissant#1018
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
| for i in */expected/*.json; do | ||
| echo testing $i | ||
| mlcroissant validate --jsonld $i | ||
| done |
There was a problem hiding this comment.
I'm just putting this at the bottom of the PR... https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-12333/1/consoleFull shows a failure to deploy the EC2 instance so I kicked off another run: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-12333/2/
There was a problem hiding this comment.
Oh good, tests are passing now: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-12333/2/testReport/
landreev
left a comment
There was a problem hiding this comment.
Looks good.
It's an impressive number of files, that needed to be changed to accommodate the fix. (which is a function of how much test coverage you have for croissant - so I mean it as a good thing, obvs.)
What this PR does / why we need it:
This commit reverts 9e619b6 from #12214. I made that change to mirror a change in the Croissant 1.1 spec but there's a mismatch between the spec change and
mlcroissant validate. I opened this issue upstream:Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Try this before and after this change (
pip install -r requirements.txtfirst)cd src/test/resources/croissant && ./validate.shThe "before" output should show an error like this for every single Croissant file:
The "after" output should look something like this. That is to say, there are no errors for non-drafts. For drafts, the errors and warnings below are expected and the same as before #12214 for Croissant 1.1 was merged:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No.
Is there a release notes update needed for this change?:
No. I looked at https://github.com/IQSS/dataverse/blob/12014-valid-croissant/doc/release-notes/12014-croissant-1.1.md from the last PR (#12214) and it's still accurate.
Additional documentation:
I updated the API changelog. You can preview the change at https://dataverse-guide--12333.org.readthedocs.build/en/12333/api/changelog.html