asadjan4611 commented on issue #17930:
URL: 
https://github.com/apache/dolphinscheduler/issues/17930#issuecomment-3938413050

   Thanks for raising this. I’d like to help on child issue #17930 
(`[Improvement][API] Add worker group id at TaskDefinition`).
   
   I think adding `workerGroupId` to task definition is the correct direction, 
and we should treat this as an ID-based migration (name -> ID) to avoid future 
rename breakage.
   
   ### Proposed approach
   
   1. **Schema/API**
      - Add `worker_group_id` in task definition model (DB + API DTO/VO).
      - Keep existing `workerGroup` (name) temporarily for compatibility during 
transition.
   
   2. **Read/Write behavior**
      - On create/update task definition:
        - Prefer `workerGroupId` if provided.
        - If only `workerGroup` (name) is provided, resolve name -> id in API 
layer.
      - On query/list:
        - Return `workerGroupId` and (optionally) resolved `workerGroupName` 
for UI display.
   
   3. **Validation**
      - Reject unknown/non-existing group id.
      - Reject groups not pre-defined in DB (aligned with DSIP-106 governance).
      - Add clear error messages for invalid mappings.
   
   4. **Backward compatibility**
      - Keep old field for one release cycle.
      - Add deprecation note in API docs.
      - Add migration SQL/script to backfill `worker_group_id` from existing 
`workerGroup` names.
   
   5. **Scheduler/worker runtime impact**
      - Internal scheduling should rely on ID for persistence and relation 
consistency.
      - Runtime dispatch can resolve ID -> effective worker-group name(s) if 
needed by current worker registration path.
   
   6. **Tests**
      - API tests: create/update/list with id-only, name-only, both fields, and 
invalid cases.
      - Migration test: existing task definitions are correctly backfilled.
      - Compatibility test: old clients (name-only payload) still work.
   
   If this direction is acceptable, I can open a PR draft with:
   - DB migration + entity change
   - API request/response updates
   - compatibility resolver logic
   - tests for both new and legacy payloads.


-- 
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]

Reply via email to