dolphinscheduler [Improvement][Spark] Support Local Spark Cluster

trafficstars

Purpose of the pull request

fixes: https://github.com/apache/dolphinscheduler/issues/15548

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

Feb 11 '24 03:02 pegasas

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 39.61%. Comparing base (e66441a) to head (d67b726).

:exclamation: Current head d67b726 differs from pull request most recent head 6329454. Consider uploading reports for the commit 6329454 to get more accurate results

Files	Patch %	Lines
.../dolphinscheduler/plugin/task/spark/SparkTask.java	53.84%	3 Missing and 3 partials :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##                dev   #15589      +/-   ##
============================================
- Coverage     39.63%   39.61%   -0.02%     
+ Complexity     5020     5016       -4     
============================================
  Files          1349     1349              
  Lines         45578    45584       +6     
  Branches       4886     4888       +2     
============================================
- Hits          18063    18059       -4     
- Misses        25595    25605      +10     
  Partials       1920     1920

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Feb 19 '24 00:02 codecov-commenter

attach test pics:

Mar 02 '24 16:03 pegasas

attach test pics:

@pegasas I don't think the Master on the ui is a required field, WDYT

Mar 04 '24 04:03 rickchengx

attach test pics:

@pegasas I don't think the Master on the ui is a required field, WDYT

Done.

I set required to false first, in the future we may link discussion here.

reference: https://spark.apache.org/docs/latest/submitting-applications.html#master-urls

In previous design, we combines logic with master & deploy-mode. But in official doc example, it comes out with master urls no matter in local/standalone/yarn/mesos/k8s.

Mar 04 '24 05:03 pegasas

Can we remove the option 'local' from deploy-mode?

We can only set cluster/client at deploy mode, the local should belong to master config.

Mar 19 '24 09:03 ruanwenjun

Please update the usage doc.

updated.

Mar 19 '24 10:03 pegasas

Can we remove the option 'local' from deploy-mode?

We can only set cluster/client at deploy mode, the local should belong to master config.

Thanks @ruanwenjun . I‘ve add TODO's in related code lines.

Mar 19 '24 10:03 pegasas

@pegasas please Run 'mvn spotless:apply' to fix the code style

Apr 11 '24 05:04 rickchengx

@pegasas please Run 'mvn spotless:apply' to fix the code style

Thanks, Done.

Apr 11 '24 05:04 pegasas

Can we remove the option 'local' from deploy-mode?

We can only set cluster/client at deploy mode, the local should belong to master config.

Maybe we should delete local option in this PR, WDYT @pegasas

Apr 12 '24 10:04 rickchengx

Can we remove the option 'local' from deploy-mode? We can only set cluster/client at deploy mode, the local should belong to master config.

Maybe we should delete local option in this PR, WDYT @pegasas

sure. corrected.

Apr 12 '24 10:04 pegasas