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

Closed
wants to merge 9 commits into 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.

Copy link

github-actions bot commented Jul 6, 2024

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the Stale label Jul 6, 2024
Copy link

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

@github-actions github-actions bot closed this Jul 13, 2024
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 Stale
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