Skip to content

[Fix-18389][DataX] Read job definition from attached resource file when custom json is empty - #18434

Open
nikhiln64 wants to merge 3 commits into
apache:devfrom
nikhiln64:Fix-18389
Open

[Fix-18389][DataX] Read job definition from attached resource file when custom json is empty#18434
nikhiln64 wants to merge 3 commits into
apache:devfrom
nikhiln64:Fix-18389

Conversation

@nikhiln64

Copy link
Copy Markdown

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

  • DataxTask.buildDataxJsonFile falls back to reading the first attached resource file via taskRequest.getResourceContext() when customConfig is set and the inline json is empty, then applies the existing placeholder substitution unchanged
  • DataxParameters.checkParameters accepts custom config with either inline json or a non empty resourceList
  • Added testCheckParametersWithCustomConfig covering inline json, resource file only, and neither

Verify this pull request

This change added tests and can be verified as follows:

  • Added testCheckParametersWithCustomConfig to DataxParametersTest. Verified the new resource file case fails against the previous implementation and passes with this change. Full dolphinscheduler-task-datax module suite passes on JDK 8 with spotless applied.

@boring-cyborg

boring-cyborg Bot commented Jul 22, 2026

Copy link
Copy Markdown

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 SbloodyS added the bug Something isn't working label Jul 22, 2026
@SbloodyS SbloodyS added this to the 3.5.0 milestone Jul 22, 2026

@SbloodyS SbloodyS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@nikhiln64

Copy link
Copy Markdown
Author

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.

@github-actions github-actions Bot added the UI ui and front end related label Jul 23, 2026
@SbloodyS

Copy link
Copy Markdown
Member

The UI and worker validation are still inconsistent for the "{}" placeholder.

DataxParameters.checkParameters() treats "{}" as an absent inline definition and rejects it when no resource is attached. However, the UI validator only checks whether model.json is blank and whether it is valid JSON. Therefore, "{}" passes the UI validation without a resource, but the task is later rejected during worker initialization.

The exact string comparison also does not recognize semantically equivalent empty objects such as { } or a formatted multi-line empty object. When a resource is attached, these values will be treated as inline JSON and the resource fallback will not be used.

Please determine whether the inline JSON is an empty object semantically instead of comparing it with the literal "{}", and apply the same rule consistently in both UI and backend validation. Please also centralize the backend check instead of duplicating it in DataxParameters and DataxTask, and add coverage for "{}", { }, and a formatted empty object, both with and without a resource.

@SbloodyS SbloodyS added the first time contributor First-time contributor label Jul 28, 2026
@nikhiln64

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working first time contributor First-time contributor test UI ui and front end related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [workflow define] Datax wants to use resource file error

2 participants