[Fix-18389][DataX] Read job definition from attached resource file when custom json is empty - #18434
[Fix-18389][DataX] Read job definition from attached resource file when custom json is empty#18434nikhiln64 wants to merge 3 commits into
Conversation
|
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
left a comment
There was a problem hiding this comment.
The resource-file fallback is not reached for the workflow reported in #18389.
The new logic reads the resource only when StringUtils.isEmpty(dataXParameters.getJson()) is true. However, the issue’s actual task parameters contain:
"customConfig": 1,
"json": "{}",
"resourceList": [...]Since "{}" is not empty, the task will continue writing {} to the generated DataX job file instead of reading the attached resource.
The current UI also requires the JSON field to be non-empty even when a resource is selected, so the resource-only case covered by DataxParametersTest cannot be created through the UI.
Please make the UI and worker follow consistent source-selection rules, handle existing tasks that use {} as the placeholder, and add a DataxTask-level regression test using a real resource file and ResourceContext. The current test only validates checkParameters() and does not verify that the resource content is written to the generated job file.
|
Thank you for the careful review @SbloodyS, all three points are addressed and pushed. The worker now treats the UI placeholder {} the same as an empty json field through a shared isInlineJsonAbsent check, so the workflow from the issue reaches the resource fallback. checkParameters follows the same rule, meaning {} plus an attached resource is valid while {} alone is still rejected. The UI json field validation in use-datax.ts now accepts an empty json editor when a resource file is selected, so the UI and worker follow the same source selection rules and the resource only case can actually be created through the UI. And there is a new DataxTask level regression test that runs handle() with customConfig, json set to {} and a real temp resource file wired through ResourceContext, asserting the generated job file contains the resource content rather than the placeholder. The full module suite passes on JDK 8 with spotless applied. The fresh CI run will need an approval when you have a moment. |
|
The UI and worker validation are still inconsistent for the
The exact string comparison also does not recognize semantically equivalent empty objects such as Please determine whether the inline JSON is an empty object semantically instead of comparing it with the literal |
…en custom json is empty
…ign UI validation, add task-level regression test
…the check in DataxParameters
|
Thanks @SbloodyS, all four points are addressed and pushed, and the branch is rebased onto the latest dev. The check is now centralized in a single DataxParameters.isInlineJsonAbsent() method that both checkParameters and DataxTask.buildDataxJsonFile call, so the duplicated literal comparison is gone. It treats the inline json as absent when it is blank or when it parses to a JSON object with no fields, so {}, { } and a formatted multi line empty object are all recognized, while malformed json still counts as present so the existing format validation reports it. The UI validator in use-datax.ts applies the same semantic rule. A parsed empty object without a resource file is now rejected with the same message as an empty editor, and it is accepted when a resource file is selected, so the UI and the worker agree on every combination. DataxParametersTest now covers null, blank, {}, { } and a formatted empty object, each both with and without a resource, plus the malformed json case and the inline definition winning when both are present. The task level test now feeds a formatted empty object through the real ResourceContext path and asserts the generated job file carries the resource content. The full module suite passes locally, 12 tests across DataxTaskTest and DataxParametersTest. |
Was this PR generated or assisted by AI?
YES. The implementation and tests were written with AI assistance (Claude Code), reviewed and driven by a human. The root cause analysis was posted on the issue before starting.
Purpose of the pull request
Fixes #18389. When a DataX task uses custom config with the job definition attached as a resource file, the worker downloads the resource but the plugin never reads it and runs with the empty inline json, so the job fails. This PR makes the plugin read the job definition from the first attached resource file whenever the inline json is empty, mirroring the pattern the SeaTunnel task plugin already uses through ResourceContext.
Brief change log
Verify this pull request
This change added tests and can be verified as follows: