qiuyanjun888 commented on issue #18166: URL: https://github.com/apache/dolphinscheduler/issues/18166#issuecomment-4766155414
Hi maintainers, I looked into this issue and the closed PR #18273. I agree that reviving the old `TaskFilesTransferUtils` / `localParams` approach as-is would repeat the design problem that was already discussed there. Here is a possible new design direction that tries to follow the feedback from #18273: ## Proposed direction Restore FILE parameter transfer as an explicit task-level file transfer feature, not as an implicit `localParams` path convention. ### 1. Add explicit task-level transfer metadata For tasks that want to produce or consume files, add a dedicated file-transfer configuration instead of relying on `localParams` values: - output mappings: `outputKey`, local source path, file/directory type, required/optional flag, and an explicit `upload` switch; - input mappings: local target path, upstream task identifier, and upstream `outputKey` to download; - transfer should be disabled by default unless the task explicitly declares it. This matches the previous maintainer suggestion of using a task-level switch plus an explicit parameter to select an upstream task output. ### 2. Keep `localParams` / varPool out of the storage-path contract `localParams` / varPool can still expose logical values to the task, but they should not be the source of truth for resource storage paths. Instead, the runtime should build a small `FileTransferContext` in `TaskExecutionContext`, containing resolved input download entries and output upload entries. This avoids coupling file movement to parameter names, file renames, or varPool merging rules. ### 3. Worker lifecycle - Before task execution: validate input target paths are inside the task working directory, resolve upstream output metadata, and download files from resource storage. - After successful task execution: validate declared output paths, upload files/directories to resource storage, then persist output metadata for downstream tasks. - On failure: do not upload incomplete outputs, and make the error explicit if a required output is missing. A deterministic storage path could be based on workflow instance / task instance / output key, for example under the existing `DATA_TRANSFER` namespace, but downstream tasks should consume it via metadata rather than by constructing the path themselves. ### 4. Compatibility and documentation I would not try to silently support every old FILE-parameter behavior in the first PR. A safer first step could be: - update the FILE parameter documentation to describe the new explicit mapping model; - preserve the user-facing goal of upstream OUT file -> downstream IN file; - avoid changing global parameter parsing rules; - fail fast with a clear message if resource storage is not configured. ### 5. Test plan A focused implementation should add tests for: - upstream task success uploads declared output files; - downstream task initialization downloads selected upstream output files; - missing required output fails clearly; - path traversal / absolute path targets are rejected; - directory output handling if directories are supported; - multiple upstream outputs do not collide. Open questions before implementation: 1. Is this explicit task-level transfer model acceptable for fixing #18166? 2. Should the metadata be persisted in an existing task-instance field / varPool-compatible structure, or should it use a dedicated runtime field/table? 3. Should the first PR implement only Shell/common task support, or should it introduce a task-plugin API that all task types can opt into later? 4. Should the current documentation be corrected immediately to say the old FILE parameter behavior is not implemented, while the new design is being built? If this direction looks reasonable, I can prepare a smaller implementation plan and tests before opening a PR. -- 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]
