SEZ9 commented on issue #11052: URL: https://github.com/apache/seatunnel/issues/11052#issuecomment-5366259519
Thanks @DanielLeens for the detailed analysis, and @davidzollo for confirming the direction. I agree with Option 1 (deprecate-first) rather than removing `Expression` outright. Since `Expression` is still a public `seatunnel-api` type — stored in `RequiredOption.ConditionalRequiredOptions` and validated through `ConfigValidator` — deleting it immediately would be a source-level breaking change for downstream extensions, even if all in-repo usages are migrated in one PR. So let's lock the first PR's scope to what @DanielLeens outlined: 1. Keep `Condition<?>` as the long-term single model. 2. Migrate the internal evaluation / rendering path toward `Condition<?>`. 3. Keep deprecated `Expression` entrypoints / constructors / accessors as forwarding compatibility shims for one release cycle. 4. Keep behavior and error-message rendering unchanged in this step. Updating the `seatunnel-engine` and `seatunnel-core` call paths in the same PR is fine as long as observable behavior stays identical, and the `Expression`-typed APIs in `ConfigValidator` and `RequiredOption` should not be removed in this first change. @nzw921rx, can you confirm you're OK taking this on with the deprecate-first scope above? Thanks all — looking forward to the PR. <!-- streview-comment:436 --> -- 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]
