wuchong commented on a change in pull request #13332:
URL: https://github.com/apache/flink/pull/13332#discussion_r490136576
##########
File path:
flink-table/flink-sql-client/src/test/resources/test-sql-client-defaults.yaml
##########
@@ -137,19 +137,10 @@ catalogs:
execution:
planner: "$VAR_PLANNER"
type: "$VAR_EXECUTION_TYPE"
- time-characteristic: event-time
Review comment:
Do not remove entries for the existing test files. The options are still
supported, so the tests are valid.
Please add a new testing yaml to verify the default behaivor.
##########
File path: flink-table/flink-sql-client/conf/sql-client-defaults.yaml
##########
@@ -100,18 +100,10 @@ execution:
planner: blink
# 'batch' or 'streaming' execution
type: streaming
- # allow 'event-time' or only 'processing-time' in sources
- time-characteristic: event-time
- # interval in ms for emitting periodic watermarks
- periodic-watermarks-interval: 200
# 'changelog', 'table' or 'tableau' presentation of results
result-mode: table
# maximum number of maintained rows in 'table' presentation of results
max-table-result-rows: 1000000
- # parallelism of the program
- parallelism: 1
Review comment:
What about only comment out (not remove) the `parallelism` and
`restart-strategy`?
1. User can still easily enable them in the `sql-client-defaults.yaml`.
2. It seems that other options (`time-characteristic`,
`periodic-watermarks-interval`, `max-parallelism`) override the default value
of underlying pipeline configuraiton. So removing them may affect the SQL job
behavior.
Besides
We may also need to update
`org.apache.flink.table.client.config.entries.ExecutionEntry#getParallelism`,
it hard code to return a default value 1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
