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

fixed The jar type of java task shell script error #15645

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

yangyanh
Copy link

@yangyanh yangyanh commented Feb 28, 2024

Purpose of the pull request

fix #15641

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@davidzollo
Copy link
Contributor

Thanks for your first contribution. Can you update the PR title in English? by the way, please add a Unit Test.

@yangyanh yangyanh changed the title 修复java节点任务的jar类型脚本问题 fixed The jar type of java task shell script error Feb 28, 2024
@qingwli
Copy link
Member

qingwli commented Feb 28, 2024

Could you raise an issue and discribe the bug, thanks.

@yangyanh
Copy link
Author

Could you raise an issue and discribe the bug, thanks.

you can read from :#15641

Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

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

Please run 'mvn spotless:apply' to fix the code style @yangyanh

@SbloodyS SbloodyS added the 3.2.2 label Feb 29, 2024
@SbloodyS SbloodyS added this to the 3.2.2 milestone Feb 29, 2024
@SbloodyS SbloodyS added the bug Something isn't working label Feb 29, 2024
@yangyanh yangyanh requested a review from rickchengx March 4, 2024 02:02
@SbloodyS SbloodyS requested a review from ruanwenjun March 5, 2024 01:38
@@ -183,9 +184,8 @@ protected String buildJarCommand() {
StringBuilder builder = new StringBuilder();
builder.append(getJavaCommandPath())
.append("java").append(" ")
.append(buildResourcePath()).append(" ")
.append(buildExtDirs()).append(" ")
Copy link
Member

Choose a reason for hiding this comment

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

Can we refactor this, use -cp rather than -jar.

$JAVA_HOME/bin/java $JVM_ARGS \
  -cp "$taskInstanceWorkingPath" \
  Main.class

Copy link
Author

Choose a reason for hiding this comment

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

If you use -jar, you need to specify the entry class, but the page cannot specify the entry class on the configuration page when you select the jar type

Copy link
Member

@ruanwenjun ruanwenjun Mar 5, 2024

Choose a reason for hiding this comment

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

Yes,I just find we cannot set entry class......
So we only support fat jar, this is sad.
So the final command should look like

$JAVA_HOME/bin/java -jar xx.jar $JVM_ARGS

Can we remove -Djava.ext.dirs, it seems not a good idea to use this param.

Copy link
Author

Choose a reason for hiding this comment

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

It seems that -jar cannot work with -cp together,-jar cannot get -cp's libraries,Is your shell worked success?have you test your shell script?

Copy link
Author

Choose a reason for hiding this comment

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

If you use -cp, you need to specify the entry class, but the page cannot specify the entry class on the configuration page when you select the jar type

Copy link
Member

@ruanwenjun ruanwenjun Mar 7, 2024

Choose a reason for hiding this comment

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

It seems that -jar cannot work with -cp together,-jar cannot get -cp's libraries,Is your shell worked success?have you test your shell script?

Yes, -jar can not work with -cp.

Once we use -jar then the jar should be a fat jar, the better way is to add ext jar by set Class-Path at MANIFEST.MF.

So Can we add entry class in UI and use -cp to refactor this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.2 backend bug Something isn't working first time contributor First-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [dolphinscheduler-task-java] The jar type task script error of the java node causes a running error
6 participants