-
Notifications
You must be signed in to change notification settings - Fork 91
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
fix: Spark with loader has dependency conflicts #480
Conversation
[Bug] Spark loader meet Exception: Class is not registered
[Bug] Spark loader meet Exception: Class is not registered
Spark loader has dependency conflicts
Codecov Report
@@ Coverage Diff @@
## master #480 +/- ##
============================================
- Coverage 62.48% 62.42% -0.07%
+ Complexity 1868 895 -973
============================================
Files 260 91 -169
Lines 9432 4404 -5028
Branches 875 519 -356
============================================
- Hits 5894 2749 -3145
+ Misses 3154 1450 -1704
+ Partials 384 205 -179
... and 169 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Spark loader has dependency conflicts
Spark loader has dependency conflicts
Spark loader has dependency conflicts
Spark loader has dependency conflicts
@imbajin @simon824 @JackyYangPassion Could you give me some advice? thanks.The error unittest I can pass in the local . |
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.
Thanks for your contribution~
Could you share some detailed context information? such as conflicting error messages.
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/spark/HugeGraphSparkLoader.java
Outdated
Show resolved
Hide resolved
could rebase/merge #478 first (will also fix the |
@haohao0103 please update the branch with the latest code & fix the conflicts & then test the bug again |
# Conflicts: # hugegraph-dist/scripts/dependency/known-dependencies.txt # hugegraph-loader/pom.xml
Spark loader has dependency conflicts
hugegraph-loader/src/main/resources/META-INF/services/shaded.jakarta.ws.rs.client.ClientBuilder
Outdated
Show resolved
Hide resolved
Spark loader has dependency conflicts
bypass server for hbase bulk delete massive vertices and edges
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/direct/loader/HBaseDirectLoader.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/direct/loader/HBaseDirectLoader.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/direct/loader/HBaseDirectLoader.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/direct/loader/HBaseDirectLoader.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/direct/loader/HBaseDirectLoader.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/constant/Constants.java
Outdated
Show resolved
Hide resolved
bypass server for hbase bulk delete massive vertices and edges
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/executor/LoadOptions.java
Outdated
Show resolved
Hide resolved
hugegraph-loader/src/main/java/org/apache/hugegraph/loader/constant/Constants.java
Outdated
Show resolved
Hide resolved
bypass server for hbase bulk delete massive vertices and edges
Hi, @haohao0103, Wouldn it be better to separate the two things in different PRs? |
@liuxiaocs7 hi ,I have revert the last few commits and will deal with it after this pr merge. |
remove incubating
Hi, @haohao0103, thanks for your work, but -1 for me. I tested locally use but got:
If I remember correctly, delimieter has a default comma, we need to check this?
|
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
shaded.org.glassfish.jersey.client.JerseyClientBuilder |
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 have a question, why do I need to add SPI to these two classes (this one and below), it seems to be able to import normally without adding these two?
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.
hello, I understand that it is because SPI added these two classes in jersey-client jar and other jars. When we do shade, we are actually changing the path information of these two classes, so we need to add SPI to these two shade classes.
hi,i have check it .when file format is json the default delimiter is null。 |
fix delimiter is null
if (Optional.ofNullable(delimiter).isPresent()) { | ||
elements = builder.build(fileSource.header(), | ||
row.mkString(delimiter).split(delimiter)); |
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.
@liuxiaocs7 could test 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
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.
we lack the test for spark-ci
indeed, so next time could add some basic UT for it?
No description provided.