njnu-seafish commented on PR #18444: URL: https://github.com/apache/dolphinscheduler/pull/18444#issuecomment-5213976833
> public API response contract is unintentionally changed > > `queryWorkflowInstanceListPaging`, `queryTopNWorkflowInstance`, and `queryByTriggerCode` now use `listSql`, but their results are returned directly by public API endpoints as `WorkflowInstance` objects. > > Consequently, these previously populated response properties will become `null` or be omitted: > > * `commandParam` > * `globalParams` > * `historyCmd` > * `varPool` > * `stateHistory` > > This is therefore not only an internal DAO optimization—it changes the existing API response contract and may break API consumers. > > Please either: > > 1. Keep using `baseSql` for queries whose entities are returned directly by public APIs; or > 2. Introduce an explicit lightweight response DTO, document the incompatible API change, and add controller-level response regression tests. > > The critical runtime queries reverted to `baseSql` look correct, but this public API compatibility issue remains blocking. Exactly. The reviewer's points are incredibly thorough. I will go with Option 2. This allows us to retain the performance optimizations of listSql while introducing explicit, lightweight DTOs to lock down the API contract. We will no longer let entity fields implicitly dictate the API response. -- 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]
