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

[ZEPPELIN-6084] Update jena-arq to 4.10 #4886

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Reamer
Copy link
Contributor

@Reamer Reamer commented Oct 28, 2024

What is this PR for?

This PR is a follow-up to #4507.
I have decided to switch to the latest version 4.x. From 5.x JDK 17 is required. Obsolete functions have been removed.

Note: I do not use the interpreter myself. I have not tested it manually either. I rely on the unit tests.

What type of PR is it?

Improvement

What is the Jira issue?

How should this be tested?

  • CI

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? Maybe
  • Does this needs documentation? No

@@ -108,6 +108,10 @@ void testSuccessfulRawQuery() {
assertEquals(Code.SUCCESS, result.code());

final String expected = "?subject\t?predicate\t?object\n" +
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently the sorting has been changed a little. I hope it doesn't cause any problems.

@Reamer
Copy link
Contributor Author

Reamer commented Nov 7, 2024

@pjfanning Can you do a review here?

@Reamer
Copy link
Contributor Author

Reamer commented Nov 12, 2024

@645775992 Can you review this PR?

@pjfanning
Copy link
Contributor

Jena is not something I know a great deal about but the changes look ok to me.

@Reamer Reamer requested a review from jongyoul November 13, 2024 07:11
@Reamer
Copy link
Contributor Author

Reamer commented Nov 24, 2024

@pan3793 @jongyoul Please review.

@Reamer Reamer requested a review from pan3793 November 24, 2024 12:01
@pan3793
Copy link
Member

pan3793 commented Nov 25, 2024

It seems that the sparql interpreter does not have integration tests, I don't use sparql, so may not be able to evaluate this change. Is it possible to leverage the testcontainers to build an integration test?

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.

3 participants