Skip to content
This repository was archived by the owner on Feb 24, 2026. It is now read-only.

fix(common): handle spring-boot2/jersey dependency conflicts#131

Merged
javeme merged 7 commits intoapache:masterfrom
z7658329:feat/commons_dependency_conflict
Apr 20, 2023
Merged

fix(common): handle spring-boot2/jersey dependency conflicts#131
javeme merged 7 commits intoapache:masterfrom
z7658329:feat/commons_dependency_conflict

Conversation

@z7658329
Copy link
Copy Markdown
Member

@z7658329 z7658329 commented Apr 7, 2023 •
edited
Loading

fix commons dependency conflicts and prevent future uncertainty issues.
also see: 445

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2023 •
edited
Loading

Codecov Report

Merging #131 (549cdc7) into master (82f2a65) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #131   +/-   ##
=========================================
  Coverage     93.15%   93.15%           
  Complexity       65       65           
=========================================
  Files             9        9           
  Lines           263      263           
  Branches         22       22           
=========================================
  Hits            245      245           
  Misses            8        8           
  Partials         10       10
Impacted Files Coverage Δ
.../java/org/apache/hugegraph/version/RpcVersion.java 50.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

javeme
javeme previously approved these changes Apr 8, 2023
View reviewed changes
imbajin
imbajin reviewed Apr 11, 2023
View reviewed changes
pom.xml

<properties>
<revision>1.0.0</revision>
<revision>1.0.1</revision>
Copy link
Copy Markdown
Member

@imbajin imbajin Apr 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

search CommonVersion class & also need update it

https://github.com/apache/incubator-hugegraph-commons/blob/82f2a6539f50cf060f02c3d8663444b14050796f/hugegraph-common/src/main/java/org/apache/hugegraph/version/CommonVersion.java#L27

btw, we need a better way to modify the version, at least add a comment in pom for new devs?

@javeme or consider get the version/revision from pom.xml rather than class property? (Or remove common version check if not necessary)

Copy link
Copy Markdown
Contributor

@javeme javeme Apr 11, 2023

Choose a reason for hiding this comment

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

If it is packaged in a jar, we cannot get the version from the pom.xml, so it fallback to the default version.

Copy link
Copy Markdown
Member

@imbajin imbajin Apr 11, 2023

Choose a reason for hiding this comment

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

If it is packaged in a jar, we cannot get the version from the pom.xml, so it fallback to the default version.

OK, we could consider these way: (way2 is the easiest)

  1. check if somewhere possible for us to put the version into the jar file (like meta file/info of a jar?)
  2. just leave a comment before revision param in pom file (devs could see the NOTE)
  3. remove the common version check, seems it's not necessary to check it?

Copy link
Copy Markdown
Member Author

@z7658329 z7658329 Apr 12, 2023

Choose a reason for hiding this comment

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

also need to update RpcVersion.java

Copy link
Copy Markdown
Contributor

@javeme javeme Apr 16, 2023

Choose a reason for hiding this comment

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

let's keep the status quo until we find a perfect way

imbajin
imbajin previously approved these changes Apr 12, 2023
View reviewed changes
hugegraph-rpc/src/main/java/org/apache/hugegraph/version/RpcVersion.java

// The second parameter of Version.of() is for all-in-one JAR
public static final Version VERSION = Version.of(RpcVersion.class, "1.0.0");
public static final Version VERSION = Version.of(RpcVersion.class, "1.0.1");
Copy link
Copy Markdown
Member

@imbajin imbajin Apr 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

TODO: we need use one version in one project (currently could keep them..)

@z7658329
Copy link
Copy Markdown
Member Author

z7658329 commented Apr 13, 2023

image

CodeQL / Analyze (java) (pull_request) Step always failed cause timeout exception (tried 4 times), is there any way to avoid such problems？
@javeme @imbajin

@imbajin
Copy link
Copy Markdown
Member

imbajin commented Apr 13, 2023 •
edited
Loading

image

CodeQL / Analyze (java) (pull_request) Step always failed cause timeout exception (tried 4 times), is there any way to avoid such problems？ @javeme @imbajin

  1. we could add a settings.xml for it to download the dependencies from different repo source
  2. we choose to use our building command instead of auto-generated cmd, and use a param to specify the repo like mvn package -? aliyun.com (consider try this)
  3. find a action to rerun it automatically (like auto retry 5 times if failed..)

javeme
javeme previously approved these changes Apr 16, 2023
View reviewed changes
pom.xml

<properties>
<revision>1.0.0</revision>
<revision>1.0.1</revision>
Copy link
Copy Markdown
Contributor

@javeme javeme Apr 16, 2023

Choose a reason for hiding this comment

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

let's keep the status quo until we find a perfect way

@z7658329
Copy link
Copy Markdown
Member Author

z7658329 commented Apr 20, 2023

image

always build error

@z7658329
Copy link
Copy Markdown
Member Author

z7658329 commented Apr 20, 2023 •
edited
Loading

image

always build error

test in local, find that the grpc-core maven-metadata.xml file does not contains version 1.28.0, seems so strange!

image

i have ask in grpc community，see 10086

@z7658329
Copy link
Copy Markdown
Member Author

z7658329 commented Apr 20, 2023

upgrade grpc-core to 1.28.1, then build success !
image

@z7658329 z7658329 dismissed stale reviews from javeme and imbajin via df74669 April 20, 2023 07:35
imbajin
imbajin approved these changes Apr 20, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

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

Good, and you could request review here

image

@z7658329 z7658329 requested a review from javeme April 20, 2023 09:24
@javeme javeme merged commit 297e491 into apache:master Apr 20, 2023
@imbajin imbajin linked an issue Jul 26, 2023 that may be closed by this pull request
[Bug] jdk8/jdk11 springboot2.X 引用hugegraph-client1.0.0 运行报错java.lang.ClassNotFoundException: jakarta.ws.rs.ProcessingException apache/hugegraph-toolchain#445
Closed
1 task
@imbajin imbajin changed the title fix commons dependency conflict fix(common): handle spring-boot2/jersey dependency conflicts Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@javeme javeme javeme approved these changes

@imbajin imbajin imbajin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] jdk8/jdk11 springboot2.X 引用hugegraph-client1.0.0 运行报错java.lang.ClassNotFoundException: jakarta.ws.rs.ProcessingException

3 participants

@z7658329 @imbajin @javeme