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

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

Merged
merged 7 commits into from
Apr 20, 2023

Conversation

z7658329
Copy link
Member

@z7658329 z7658329 commented Apr 7, 2023

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

@codecov
Copy link

codecov bot commented Apr 7, 2023

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
@@ -89,7 +89,7 @@
</scm>

<properties>
<revision>1.0.0</revision>
<revision>1.0.1</revision>
Copy link
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.

search CommonVersion class & also need update it

public static final Version VERSION = Version.of(CommonVersion.class, "1.0.0");

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
Contributor

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
Member

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
Member Author

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
Contributor

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
@@ -24,5 +24,5 @@ public class RpcVersion {
public static final String NAME = "hugegraph-rpc";

// 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
Member

@imbajin imbajin Apr 12, 2023

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
Member Author

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
Member

imbajin 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

  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
@@ -89,7 +89,7 @@
</scm>

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

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
Member Author

image

always build error

@z7658329
Copy link
Member Author

z7658329 commented Apr 20, 2023

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
Member Author

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
Copy link
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 changed the title fix commons dependency conflict fix(common): handle spring-boot2/jersey dependency conflicts Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants