[Feature-16699][Task Plugin] Flink submit option parameter supports placeholder substitution #16703
Conversation
hanhanzhang
requested review from
caishunfeng,
SbloodyS and
Gallardot
as code owners
|
Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md) |
SbloodyS
added
feature
|
ok, can you guide me on how to add doc？ |
You should add it to the following files: |
…hang/dolphinscheduler into feature_option_parameter_replace
docs/docs/en/guide/task/flink.md
Outdated
| @@ -37,7 +37,7 @@ Flink task type, used to execute Flink programs. For Flink nodes: | |||
| | Parallelism | Used to set the degree of parallelism for executing Flink tasks. | | |||
| | Yarn queue | Used to set the yarn queue, use `default` queue by default. | | |||
| | Main program parameters | Set the input parameters for the Flink program and support the substitution of custom parameter variables. | | |||
| | Optional parameters | Set the flink command options, such as `-D`, `-C`, `-yt`. | | |||
| | Optional parameters | Set the flink command options, such as `-D`, `-C`, `-yt`, and support the substitution of custom parameter variables. | | |||
There was a problem hiding this comment.
Please add some examples of how to use it.
| @@ -37,7 +37,7 @@ Flink task type, used to execute Flink programs. For Flink nodes: | |||
| | Parallelism | Used to set the degree of parallelism for executing Flink tasks. | | |||
| | Yarn queue | Used to set the yarn queue, use `default` queue by default. | | |||
| | Main program parameters | Set the input parameters for the Flink program and support the substitution of custom parameter variables. | | |||
| | Optional parameters | Set the flink command options, such as `-D`, `-C`, `-yt`. | | |||
| | Optional parameters | Set the flink command options, such as `-D`, `-C`, `-yt`, and support the substitution of custom parameter variables, such as `-Dyarn.application.name=${job_name}` custom parameter job _name will be replaced. | | |||
There was a problem hiding this comment.
Is this custom parameter job_name setting by users with hard code? If so, this is meaningless. We should add separate field options for users to use instead of using custom parameters, which are suitable for parameter passing and global parameters.
There was a problem hiding this comment.
Not hard-coded, user custom parameter pass and replace, like main program parameters:
There was a problem hiding this comment.
As I said that Custom parameters are suitable for parameter passing between tasks in a workflow and global parameters. yarn.application.name is a parameter of task level with different functions. We should not add it here.
There was a problem hiding this comment.
I understand boundary with custom parameters, option parameter replacement is also specific to Flink Task.
There was a problem hiding this comment.
option parameter replacement is also specific to Flink Task.
I don't think we should confuse usage of custom parameters with task parameters. Sorry, I'm -1 on this.
There was a problem hiding this comment.
Understand your idea, but i have a question: custom parameters can't be used as task parameters? Now flink task main parameters can be replaced with custom parameters.
There was a problem hiding this comment.
Understand your idea, but i have a question: custom parameters can't be used as task parameters? Now flink task main parameters can be replaced with custom parameters.
I didn't say that custom parameters can't be used as task parameters. What I mean is job_name should be added into task parameters instead of custom parameters.
After take a look at dev branch, I found that the job_name already exists in task params right now with org.apache.dolphinscheduler.plugin.task.flink.FlinkParameters#appName. But it is only used in flink sql. We can just simpliy add it to the flink jar mode. And this issue can be easily solved.
Purpose of the pull request
Flink submit option parameter supports placeholder substitution
close #16699
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)
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