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: assemble executable jar for pd and store #2289

Merged
merged 9 commits into from
Aug 29, 2023

Conversation

VGalaxies
Copy link
Contributor

@VGalaxies VGalaxies commented Aug 19, 2023

Purpose of the PR

fix #2275

refer to https://hugegraph.feishu.cn/wiki/UfL0w2m8Aiekl9kHsedc8jQKnN7#QuiKdmz2ZoY3WMxqkiaczvc6nKb

Main Changes

Due to the change of hugegraph-pd's parent from spring-boot-starter-parent to hugegraph, and hugegraph's parent being apache, along with modifications to the artifactId of certain sub-modules (for example, hugegraph-pd to hg-pd-service), the following steps need to be taken:

  1. Modify the descriptor of the maven-assembly-plugin, which involves adjusting the collected jar package names accordingly.
  2. Update the corresponding jar package names in the startup script.
  3. Since hugegraph-pd is started using Spring Boot, the spring-boot-maven-plugin needs to be reconfigured.

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects (typed here)

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@imbajin
Copy link
Member

imbajin commented Aug 21, 2023

could also refer the toolchain PR: (Although not same)
apache/incubator-hugegraph-toolchain@025c5ab#diff-a97944bbb4e440a8f7f8ef46f83479ecaa1cd8da2fb1e88437b2acab9d8f842f

Comment on lines 141 to 153
<classifier>spring-boot</classifier>
<mainClass>
org.apache.hugegraph.pd.boot.HugePDServer
</mainClass>
</configuration>
<goals>
<goal>repackage</goal>
</goals>
Copy link
Member

Choose a reason for hiding this comment

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

we need some comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

refer to docs.spring.io/spring-boot/docs/current/maven-plugin/reference/htmlsingle/#packaging

fine, better to add url refer for it in future, currently leave this in PR context

@imbajin imbajin added this to the 1.5.0 milestone Aug 29, 2023
@VGalaxies VGalaxies requested a review from imbajin August 29, 2023 08:25
@@ -114,8 +114,8 @@ fi
echo "Starting HugeGraphPDServer..."

# Turn on security check
exec ${JAVA} ${JAVA_OPTIONS} -jar -Dspring.config.location=${CONF}/application.yml \
${LIB}/hugegraph-pd-*.jar >> ${OUTPUT} 2>&1 &
exec ${JAVA} -Dname="HugeGraphPd" ${JAVA_OPTIONS} -jar \
Copy link
Member

@imbajin imbajin Aug 29, 2023

Choose a reason for hiding this comment

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

Suggested change
exec ${JAVA} -Dname="HugeGraphPd" ${JAVA_OPTIONS} -jar \
exec ${JAVA} -Dname="HugeGraphPD" ${JAVA_OPTIONS} -jar \

TODO:
@VGalaxies we could better get the jar version for the suffix of name, like HugeGraphServer-1.0 | HugeGraphPD-1.5 to let users know the version directly

@imbajin imbajin merged commit c70a294 into apache:pd-store Aug 29, 2023
1 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants