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.

Sign up for GitHub

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

Jump to bottom

activate cql tests for cassandra 4 [tp-tests] #3033

Merged
merged 1 commit into from
Sep 26, 2022
Merged

activate cql tests for cassandra 4 [tp-tests] #3033

merged 1 commit into from
Sep 26, 2022

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented May 8, 2022 •
edited
Loading

Fixes #3017

  • Upgrade Cassandra testing dist version to 4.0.6
  • Enable full dist builds on java 11 tests
  • Fix log collection during local testing

Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there an issue associated with this PR? Is it referenced in the commit message?
  • Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Is your initial contribution a single, squashed commit?

For code changes:

  • Have you written and/or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

@janusgraph-bot janusgraph-bot added the cla: external Externally-managed CLA label May 8, 2022
FlorianHockmann
FlorianHockmann requested changes May 10, 2022
View reviewed changes
janusgraph-dist/pom.xml Outdated Show resolved Hide resolved
janusgraph-dist/src/assembly/static/cassandra/conf/jvm-server.options Show resolved Hide resolved
janusgraph-dist/src/assembly/static/bin/janusgraph.sh Outdated Show resolved Hide resolved
janusgraph-dist/pom.xml Outdated Show resolved Hide resolved
@porunov
Copy link
Member

porunov commented May 10, 2022

@farodin91 Thank you for the fixes! Do you think it's possible to split Cassandra version update (which I think is related to #2325) and dist tests fix (which is related to #3017) to 2 different PRs? I think we need to fix dist tests bug in v0.6 branch. Otherwise we won't be able to release 0.6.x versions anymore.

@FlorianHockmann FlorianHockmann mentioned this pull request May 11, 2022
Merged
9 tasks
@farodin91 farodin91 marked this pull request as draft May 17, 2022 08:29
@farodin91 farodin91 marked this pull request as ready for review June 9, 2022 06:53
@farodin91
Copy link
Contributor Author

@FlorianHockmann Would you like to review?

@FlorianHockmann
Copy link
Member

Would you like to review?

I just tried to review this again, but I'm still a bit confused: Is this now the PR to update Cassandra to version 4? In that case, this also fixes #2325 as @porunov noted, but then it should definitely be clearly documented in the changelog.

@li-boxuan
Copy link
Member

I have a dumb question: when we say "java 11 support" do we use it at compile time or runtime or both?

@farodin91
Copy link
Contributor Author

runtime and compile and not source

@porunov
Copy link
Member

porunov commented Jun 9, 2022

I just tried to review this again, but I'm still a bit confused: Is this now the PR to update Cassandra to version 4? In that case, this also fixes #2325 as @porunov noted, but then it should definitely be clearly documented in the changelog.

It looks like this PR upgrades only distribution version of Cassandra to Cassandra 4. In pom.xml we still use Cassandra 3.

@farodin91
Copy link
Contributor Author

cql hadoop can't be replaced directly. We have to switch to the spark read by datastax.

@farodin91 farodin91 changed the title fix dists full on java 11 activate cql tests for cassandra 4 Jun 25, 2022
FlorianHockmann
FlorianHockmann reviewed Jun 27, 2022
View reviewed changes
docs/changelog.md Outdated Show resolved Hide resolved
docs/changelog.md Outdated Show resolved Hide resolved
@farodin91 farodin91 force-pushed the test-dists-full branch 7 times, most recently from c7d1619 to 62723d6 Compare September 17, 2022 07:45
@farodin91 farodin91 added this to the Release v1.0.0 milestone Sep 17, 2022
@farodin91 farodin91 changed the title activate cql tests for cassandra 4 activate cql tests for cassandra 4 [tp-tests]` Sep 17, 2022
@farodin91 farodin91 changed the title activate cql tests for cassandra 4 [tp-tests]` activate cql tests for cassandra 4 [tp-tests] Sep 17, 2022
@farodin91 farodin91 force-pushed the test-dists-full branch 3 times, most recently from ab4c641 to e056b1b Compare September 18, 2022 11:18
@farodin91
Copy link
Contributor Author

@FlorianHockmann Would you like to review it again?

@farodin91
Copy link
Contributor Author

@porunov Would you like to review this PR?

porunov
porunov reviewed Sep 19, 2022
View reviewed changes
Copy link
Member

@porunov porunov left a comment

Choose a reason for hiding this comment

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

Thank you @farodin91 ! I have one comment below

janusgraph-dist/src/assembly/static/cassandra/bin/cassandra Show resolved Hide resolved
porunov
porunov approved these changes Sep 20, 2022
View reviewed changes
Copy link
Member

@porunov porunov left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @farodin91 !

FlorianHockmann
FlorianHockmann reviewed Sep 22, 2022
View reviewed changes
.github/workflows/ci-backend-cql.yml Show resolved Hide resolved
docs/changelog.md Outdated Show resolved Hide resolved
@farodin91
activate cql tests for cassandra 4 [tp-tests]
e78dca9 
* activate java 11 for full distrubtion tests
* update full distrubtion version of cassandra 4.0.6
* add java 11 tests for cassandra 4.0.6
* get cassandra logs for dist tests

Signed-off-by: Jan Jansen <jan.jansen@gdata.de>
@farodin91
Copy link
Contributor Author

@FlorianHockmann I've fixed both comments.

@farodin91 farodin91 merged commit 50a6d1b into JanusGraph:master Sep 26, 2022
@farodin91 farodin91 deleted the test-dists-full branch September 26, 2022 07:05
@farodin91 farodin91 mentioned this pull request Nov 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@porunov porunov porunov approved these changes

@FlorianHockmann FlorianHockmann FlorianHockmann approved these changes

@li-boxuan li-boxuan Awaiting requested review from li-boxuan

Assignees
No one assigned
Labels
cla: external Externally-managed CLA
Projects
None yet
Milestone
Release v1.0.0
Development

Successfully merging this pull request may close these issues.

Dist tests failing
5 participants
@farodin91 @porunov @FlorianHockmann @li-boxuan @janusgraph-bot