Skip to content

fix: assemble executable jar for pd and store#2289

Merged
imbajin merged 9 commits intoapache:pd-storefrom
VGalaxies:pd-store
Aug 29, 2023
Merged

fix: assemble executable jar for pd and store#2289
imbajin merged 9 commits intoapache:pd-storefrom
VGalaxies:pd-store

Conversation

@VGalaxies
Copy link
Copy Markdown
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
Copy Markdown
Member

imbajin commented Aug 21, 2023

could also refer the toolchain PR: (Although not same)
apache/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
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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
# 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
Copy Markdown
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
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