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

[DSIP-27][Task Plugin] Some improvements of JAVA task plugin #16542

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

Conversation

ailiujiarui
Copy link

@ailiujiarui ailiujiarui commented Aug 29, 2024

Purpose of the pull request

Update the Java task plugin
close:#15819

Brief change log

  • Deprecate write java code in JAVA task
  • Rename the JAR to FATJAR
    Only the displayed name has been modified, the functionality remains unchanged.
  • Add the new type NORMALJAR
    Allow users to upload normal type of jar files, which require external libraries to run properly. Users do not need to package all libraries and runtime files into a fat jar each time, making the submission of jar files more flexible and convenient.
  • Update the tests
    Delete tests about JAVA type and add the test of NORMAL JAR type
  • Update the Javadoc comment
  • Remove the Java task type tests from the e2e tests
    Since the original workflowJavaTaskE2ETest was used to test Java code types under Java task types, and this feature has been removed, this test is no longer needed.

type of FATJAR
be51083587282062ceda775cd255c1f

type of NORMALJAR
b06bccb0e0ebeaacd249a8000cf7a10

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)

Pull Request Notice

Pull Request Notice

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

@github-actions github-actions bot added UI ui and front end related backend test labels Aug 29, 2024
@SbloodyS SbloodyS changed the title [Feature-15819][Task Plugin]Update the JAVA task [DSIP-15819][Task Plugin] Update the JAVA task Aug 29, 2024
@SbloodyS SbloodyS added first time contributor First-time contributor feature new feature labels Aug 29, 2024
@SbloodyS SbloodyS changed the title [DSIP-15819][Task Plugin] Update the JAVA task [DSIP-27][Task Plugin] Update the JAVA task Aug 30, 2024
@SbloodyS SbloodyS added the DSIP label Aug 30, 2024
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts first.

@fuchanghai
Copy link
Member

image Please check if additional explanation is needed

@ailiujiarui
Copy link
Author

Hi everyone, could you please let me know if there’s anything that needs improvement? Thanks!

@SbloodyS SbloodyS changed the title [DSIP-27][Task Plugin] Update the JAVA task [DSIP-27][Task Plugin] Some improvements of JAVA task Sep 23, 2024
@SbloodyS SbloodyS changed the title [DSIP-27][Task Plugin] Some improvements of JAVA task [DSIP-27][Task Plugin] Some improvements of JAVA task plugin Sep 23, 2024
@github-actions github-actions bot added CI&CD e2e e2e test labels Sep 26, 2024
Comment on lines -127 to -128
- name: WorkflowJavaTaskE2ETest
class: org.apache.dolphinscheduler.e2e.cases.WorkflowJavaTaskE2ETest
Copy link
Member

Choose a reason for hiding this comment

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

Why delete this? CI-related content should not be deleted in PR in general.

Copy link
Author

Choose a reason for hiding this comment

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

Since the original workflowJavaTaskE2ETest was used to test Java code types under Java task types, and this feature has been removed, this test is no longer needed.

Copy link
Member

Choose a reason for hiding this comment

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

You should modify it base on the new logic according to the previous test case, not delete it.

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

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

Please change the docs too.

@ailiujiarui
Copy link
Author

I am not sure what caused the bc38b84 CI test error

@ailiujiarui
Copy link
Author

请先解决冲突。

Conflict has been resolved.

Copy link

sonarcloud bot commented Oct 13, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@SbloodyS
Copy link
Member

Document has been updated.

I didn't see any docs in this PR. Please check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend CI&CD DSIP e2e e2e test feature new feature first time contributor First-time contributor miss:docs missing documents in PR test UI ui and front end related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants