davidzollo commented on PR #10799:
URL: https://github.com/apache/seatunnel/pull/10799#issuecomment-4470231524

   Hi @vsantonastaso — thanks again for the amount of work you have put into 
this PR.
   
   I want to refine one part of my earlier framing so that my review is as 
accurate and fair as possible on the latest head.
   
   On the current revision, I do see meaningful progress on the 
checkpoint/savepoint compatibility side: the V1/V2 serializer handling, the 
legacy `TableId -> TableIdentifier` migration path, and the dedicated 
compatibility tests all move the code in a much better direction. So I do not 
want to overstate those earlier concerns as if nothing changed.
   
   That said, I still think the main architectural point needs to stay visible:
   
   I do not think this PR, in its current shape, fully demonstrates true 
per-connector Debezium version independence yet.
   
   What I mean by that is:
   
   - `connector-cdc-base` is still not fully Debezium-neutral. There are still 
places in the base layer that depend directly on `io.debezium.*` types or 
behavior, so the abstraction boundary is not completely clean yet.
   - Some of the current cross-boundary contracts are still effectively 
version-coupled in practice. They may be wrapped behind SPI interfaces, but 
they do not yet prove that different connector modules can evolve against 
materially different Debezium versions without the base module becoming the 
compatibility anchor again.
   - The patched Debezium-side classes and the remaining Debezium-aware pieces 
in the base module also suggest that this is not yet the final isolation model.
   
   So my current view is:
   
   This PR looks much more like an important foundation step than a finished 
end-state for multi-version Debezium isolation.
   
   I think that is still valuable. In fact, it is meaningful progress. But I 
would be more comfortable if we describe the result in that narrower and more 
precise way:
   
   - this PR moves the project toward per-connector Debezium version management,
   - it improves the internal migration/compatibility story,
   - and it lays groundwork for future isolation,
   - but it does not yet fully prove the final structural property of 
"independent Debezium version evolution per connector in the same runtime".
   
   In other words, my concern is now less about the specific byte-level fixes 
(which I can follow up on separately if needed), and more about whether the 
architectural claim is supported by the current boundary.
   
   If the project is comfortable landing this as a staged foundation, I think 
the PR description and follow-up expectations should reflect that explicitly.
   
   If, however, the expectation is that this PR by itself already establishes 
the full isolation story — for example, making a future PostgreSQL Debezium 
upgrade independent from a MySQL connector that stays on an older line — then I 
do not think that proof is complete yet.
   
   I hope this makes my position clearer. I am not trying to dismiss the 
progress here; I just want the review language to match the latest code and the 
actual architectural boundary as precisely as possible.


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