Skip to content
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

Issue 373, v2: Upgrade rdf-toolkit and Java #395

Closed
wants to merge 6 commits into from

Conversation

ajnelson-nist
Copy link
Contributor

@ajnelson-nist ajnelson-nist commented Jun 1, 2022

This Pull Request will resolve all requirements of Issue 373.

This PR adjusts the strategy of v1, PR 386, due to issues having been found when running a normalized file through rdf-toolkit multiple times. Issue documentation is on CASE-Examples PR 75. pre-commit usage is removed from v2 of resolving Issue 373.

Review steps taken:

  • Pull request is against correct branch
  • CI passes in (CASE/UCO) feature branch
  • CI passes in (CASE/UCO) current unstable branch (merge-commit)
  • Impact on SHACL validation reviewed for CASE-Examples
  • Impact on SHACL validation remediated for CASE-Examples
  • Impact on SHACL validation reviewed for casework.github.io
  • Impact on SHACL validation remediated for casework.github.io

The landing page for the setup-java action states the following:

> NOTE: Adopt OpenJDK got moved to Eclipse Temurin and won't be updated
> anymore. It is highly recommended to migrate workflows from adopt to
> temurin to keep receiving software and security updates. See more
> details in the Good-bye AdoptOpenJDK post.

References:
* https://github.com/actions/setup-java/#supported-distributions
* https://blog.adoptopenjdk.net/2021/08/goodbye-adoptopenjdk-hello-adoptium/

Signed-off-by: Alex Nelson <[email protected]>
(cherry picked from commit 2b15709)
With the current version of `rdf-toolkit-action`, this is expected to
cause a CI failure due to an `rdf-toolkit.jar` incompatibility.  A
release coming soon for `rdf-toolkit-action` will update the `.jar` file
used.

Signed-off-by: Alex Nelson <[email protected]>
(cherry picked from commit 282468f)
@ajnelson-nist ajnelson-nist marked this pull request as draft June 1, 2022 22:50
@ajnelson-nist ajnelson-nist linked an issue Jun 1, 2022 that may be closed by this pull request
14 tasks
The version of rdf-toolkit run by the pre-commit hook now handles an RDF
list differently than the prior version of rdf-toolkit.

This commit is a mechanically-produced result.

References:
* #373

Signed-off-by: Alex Nelson <[email protected]>
(cherry picked from commit ab0aff8)
Copy link
Contributor

@plbt5 plbt5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The update changes all ordered lists into sets of IRIs. The application can only select one set element.

Copy link
Contributor

@plbt5 plbt5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm correct, the wget on ...rdf-toolkit-build/lastSuccessfulBuild/... might pull a new version in the future. This demands the new rdf-toolkit version to be backwards compatible with the current one to prevent the tool to potentially crash.

At the same time, constraining the toolkit with a SHA512 to one single version only, prevent new versions to be applied, and might make the tool to stall for different builds.

I assume this to be the intended behaviour.

@ajnelson-nist
Copy link
Contributor Author

If I'm correct, the wget on ...rdf-toolkit-build/lastSuccessfulBuild/... might pull a new version in the future. This demands the new rdf-toolkit version to be backwards compatible with the current one to prevent the tool to potentially crash.

At the same time, constraining the toolkit with a SHA512 to one single version only, prevent new versions to be applied, and might make the tool to stall for different builds.

I assume this to be the intended behaviour.

@plbt5 , you're right that this was the behavior I intended to encode. This download strategy hasn't changed since CASE's initial implementation of it.

However, you've made me realize that it assumes stability in the download, and that assumption is incorrect. The hard-coded SHA-512 "Pins" the expected JAR file to today's release. So, if a new release happens, the download will fail to verify.

I'll push another patch to revise the strategy to use CASE's redistribution rather than retrieving the original file. I think a preferable remediation would be accessing a version somehow "Pinned" upstream, but that's out of scope of this PR.

This prevents a potential build failure scenario where rdf-toolkit
issues a release and suddenly all CI pinned to 1.11.0's SHA-512 fails.

References:
* #373

Reported-by: Paul Brandt <[email protected]>
Signed-off-by: Alex Nelson <[email protected]>
References:
* #373

Signed-off-by: Alex Nelson <[email protected]>
@ajnelson-nist
Copy link
Contributor Author

The update changes all ordered lists into sets of IRIs. The application can only select one set element.

I'm not quite sure I follow this remark - the syntactic style of the ordered lists is changed, but they have not been converted into sets. ( ... ) is Turtle shorthand for an RDF List. Or is there something I'm missing?

Copy link
Contributor

@plbt5 plbt5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite so!

@ajnelson-nist
Copy link
Contributor Author

Closing as superseded by PR 398.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants