-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update TinkerPop 3.6.1 [tp-tests] #3208
Conversation
692d637
to
5a46f65
Compare
pom.xml
Outdated
<slf4j.version>1.7.36</slf4j.version> | ||
<logback.version>1.2.11</logback.version> | ||
<httpcomponents.httpclient.version>4.5.13</httpcomponents.httpclient.version> | ||
<httpcomponents.httpcore.version>4.4.15</httpcomponents.httpcore.version> | ||
<hadoop2.version>2.8.5</hadoop2.version> | ||
<hadoop2.version>3.3.4</hadoop2.version> |
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.
(nitpick) I think it makes sense to call this version hadoop3
now instead of hadoop2
. I.e.:
<hadoop3.version>3.3.4</hadoop3.version>
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.
That's a nitpick, so not necessary to resolve, we can refactor it afterwards
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.
I will clean before marking the pr ready.
abdd217
to
74a2cd3
Compare
9387232
to
a83351d
Compare
I will split this update into multiple PRs. |
4ed1715
to
5f1c4a6
Compare
@porunov Hbase upgrade is a pain. |
I imagine it is ... |
@porunov @FlorianHockmann @li-boxuan @rngcntr Does any have a bit time to look into hbase hadoop tests? |
It looks like this one of the bugs which stoping me to get this running fine. https://github.com/apache/hbase/pull/4819/files Other tests in apache projects were deactivated in the combination to test hbase 2 with hadoop 3. https://github.com/apache/ranger |
TP tests are skipped It would be nice to run it by adding |
@mad I would like to find a way to fix the hbase test before hand. |
Some info about hbase issue Scan for janusgraph hbase return some data, but metadata say no data exists Scan response
Metadata response
STARTKEY and ENDKEY are empties. So, that lead to empty inputSplits here org.janusgraph.hadoop.formats.hbase.HBaseBinaryInputFormat#getSplits |
@mad any idea why metadata is empty? |
Actually root cause is spark https://spark.apache.org/docs/3.2.0/core-migration-guide.html#upgrading-from-core-31-to-32
So, just put |
@mad Thank you. |
...src/main/java/org/janusgraph/graphdb/tinkerpop/optimize/strategy/JanusGraphStepStrategy.java
Show resolved
Hide resolved
@mad Updated |
Fixes JanusGraph#3069 * Upgrade hadoop 3.x * Remove Jackson 1 Signed-off-by: Jan Jansen <jan.jansen@gdata.de>
3.6 has some breaking changes https://github.com/apache/tinkerpop/blob/master/CHANGELOG.asciidoc - (breaking) mark Do we need to take this into account? |
@mad All breaking changes, i've handled multiple breaking changes. https://issues.apache.org/jira/browse/TINKERPOP-2507 and gryo removal. |
@porunov Would you like to review it again? |
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.
LGTM. Thank you @farodin91 !
Fixes #3069
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:
master
)?For code changes:
For documentation related changes: