uranusjr commented on code in PR #74538:
URL: https://github.com/apache/airflow/pull/74538#discussion_r4344092454
##########
java-sdk/sdk/src/main/kotlin/org/apache/airflow/sdk/execution/Serde.kt:
##########
@@ -156,7 +159,11 @@ private fun serializeTask(
// __type encoding is stripped. If core grows a task-level
fill_config_defaults,
// every SDK has to keep explicitly set values instead, or an explicit
retries=0
// reads as unset and picks up the configured default.
- def.configValues.forEach { (key, value) ->
+ // The Dag's queue is merged in first, so one equal to the schema default is
left out too.
+ // A trigger task takes it as well, because the Java runtime runs it.
+ val config =
+ if (dagQueue == null || "queue" in def.configValues) def.configValues else
def.configValues + ("queue" to dagQueue)
+ config.forEach { (key, value) ->
Review Comment:
```suggestion
// A trigger task takes the Dag's queue too, because the Java runtime runs
it.
val config =
if (dagQueue == null || "queue" in def.configValues) def.configValues
else def.configValues + ("queue" to dagQueue)
// Emit only config entries that differ from their schema default,
mirroring.
config.forEach { (key, value) ->
```
It may also be a good idea to move the first three lines (the comment and
the `val config =` statement) _above_ the comment block around L156-L161; that
comment belongs more to the `forEach` call.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
