Do we have some example tests that we can make more thorough so they will be reasonably exhaustive?
On Mon, Jan 2, 2023 at 1:54 PM Reuven Lax <re...@google.com> wrote: > Be very careful with the auto schema stuff around Avro. These classes > dynamically inspect Avro-generated classes (to codegen schema accessors) so > it will be easy to break this in a way that is not seen at compile time. > > On Mon, Jan 2, 2023 at 12:17 PM Alexey Romanenko <aromanenko....@gmail.com> > wrote: > >> Here is the recent update on the progress for this topic. >> >> After receiving a feedback on the design document [1] presented to >> community before and having the several discussions after (many thanks for >> this!), it was decided to take an “*option 4*” (*“Move Avro from “core” >> to generic Avro extensions using multiple Avro version specific adapters to >> handle breaking changes”*) as a way to move forward. >> >> We created an umbrella issue to track the progress [2] and the* first >> step* (“*Create Avro extension for Java SDK*”) of this [3] is already >> finished and merged. This new created extension (“ >> *sdks/java/extensions/avro/*") replicates the same Avro support >> behaviour as it's currently implemented in Java SDK “*core*”. It >> required almost no changes for the current user API (only relaxation of >> access modifiers for several class members and methods to provide an access >> from other packages to them), so it should *not* introduce any potential >> breaking changes for users, especially if they still use the current Beam >> Avro's version (1.8.2). >> >> The *next step* will be to switch all Beam Java modules to use the new >> Avro extension instead of using the “core” Avro classes. Again, we don’t >> expect any user API breaking changes for this step. >> >> *Note*: As a price for smooth and not breakable transition, we have to >> support two equal versions of Beam Avro's code (in “*core*" and in “ >> *extensions/avro*”) until the old code will be deprecated (it’s expected >> to be the *third step*). So, till this, please apply your Java SDK >> Avro-related changes (if any) in two places to keep them in sync. >> >> >> Also, please, share any of your feedback, questions, ideas or concerns on >> this topic. >> >> >> [1] >> https://docs.google.com/document/d/1tKIyTk_-HhkmVuJsxvWP5eTELESpCBe_Vmb1nJ3Ia34/edit?usp=sharing >> [2] https://github.com/apache/beam/issues/24292 >> [3] https://github.com/apache/beam/issues/24293 >> >> — >> Alexey >> >> >> >> On 18 Nov 2022, at 15:56, Alexey Romanenko <aromanenko....@gmail.com> >> wrote: >> >> Since there are no principal objections against the proposed option 2 >> (extract Avro-related code from “core” to Avro extension but keep it in >> “core” for some time because of transition period), then we will try to >> move forward and take this path. >> >> I’m pretty sure that we will face some hidden issues while working on >> this, so I’ll keep you posted =) >> >> — >> Alexey >> >> On 11 Nov 2022, at 18:05, Austin Bennett <aus...@apache.org> wrote: >> >> @Moritz: I *think* should be fine, and don't have anything specific to >> offer for what might go wrong throughout the process. :-) :shrug: >> >> >> >> On Fri, Nov 11, 2022 at 2:07 AM Moritz Mack <mm...@talend.com> wrote: >> >>> Thanks a lot for the feedback so far! I can only second Alexey. It was >>> painful to come to realize that the only feasible option seems to be >>> copying a lot of code during the transition phase. >>> >>> For that reason, it will be critical to be disciplined about the removal >>> of the to-be deprecated code in core and, ahead of time, agree on when to >>> remove it again. Any thought on how long the transition phase should be? >>> >>> >>> >>> *I am concerned of what could go wrong for users in the >>> in-between/transition state while more slowly transitioning avro to >>> extension.* >>> >>> >>> >>> @Austin Do you have any specific concern in mind here? >>> >>> To minimize this risk, we propose that all APIs should be kept as is to >>> make the migration as easy as possible and kick off with the Avro version >>> used in core. The only thing that changes will be package names. >>> >>> >>> >>> / Moritz >>> >>> >>> >>> On 10.11.22, 22:46, "Kenneth Knowles" <k...@apache.org> wrote: >>> >>> >>> >>> Thank you for writing this document. It really helps to understand the >>> options. I agree that option 2 (make a new extension and deprecate from >>> core) seems best. I think +Reuven Lax might have the most context on any >>> technical issue we will >>> >>> Thank you for writing this document. It really helps to understand the >>> options. I agree that option 2 (make a new extension and deprecate from >>> core) seems best. I think +Reuven Lax <re...@google.com> might have the >>> most context on any technical issue we will encounter around schema codegen. >>> >>> >>> >>> Kenn >>> >>> >>> >>> On Thu, Nov 10, 2022 at 7:24 AM Alexey Romanenko < >>> aromanenko....@gmail.com> wrote: >>> >>> Personally, I think that keeping two mostly identical versions of >>> Avro-related code in two different places (“core" and "extension") is rathe >>> bad practice, especially, in case of need to fix some issues there - >>> though, it’s a very low risk there since this code is quite mature and it’s >>> not touched often. On the other hand, it should give time for users >>> (several Beam releases) to update their code and use Avro from extension >>> artifact instead of core. >>> >>> >>> >>> Though, if we accept that this breaking change at compile time is >>> allowable, then this process of transition should be much faster and can be >>> performed within only one Beam release. Our main concern here is runtime >>> breaking changes that we can miss but *must* be avoided by all means. >>> >>> >>> >>> — >>> >>> Alexey >>> >>> >>> >>> On 9 Nov 2022, at 18:47, Austin Bennett <aus...@apache.org> wrote: >>> >>> >>> >>> Being tied to a specific version of a dependency, and esp. one that is >>> not-[actually-long-term]critical, sounds like a problem. It doesn't seem >>> like Avro needs to be in core. I am in favor of about any path someone >>> wants to address towards removing that from core [ *#2 in the design >>> doc seems reasonable* ]. >>> >>> >>> >>> Naturally, having ways to more easily change versions [esp. to remediate >>> CVEs, but for any specific reason ], seems very valuable. >>> >>> >>> >>> It reads as a significant problem; I wouldn't take issue with a breaking >>> [ compile time ] change, if that got things addressed and somewhat >>> straightforwardly - *I am concerned of what could go wrong for users in >>> the in-between/transition state while more slowly transitioning avro to >>> extension.* >>> >>> >>> >>> On Wed, Nov 9, 2022 at 5:43 AM Alexey Romanenko < >>> aromanenko....@gmail.com> wrote: >>> >>> Any thoughts on this? For now, we'd need to decide which path finally to >>> take to move forward. >>> >>> >>> >>> Thanks in advance! >>> >>> >>> >>> — >>> >>> Alexey >>> >>> >>> >>> On 4 Nov 2022, at 16:44, Alexey Romanenko <aromanenko....@gmail.com> >>> wrote: >>> >>> >>> >>> Hi all, >>> >>> >>> >>> Following-up an Avro dependency update discussion [1] that showed a lot >>> of uncertainties to move forward, Moritz and I decided to create a design >>> document [2] with potential options, that we believe, can be considered and >>> used further. Unfortunately, all solutions lead to breaking changes in some >>> way, though, for some of them the negative effect can be reduced by >>> preparing users for this in advance and make this transition smoother. >>> >>> >>> >>> Please, take a look on this doc and leave your comments and opinions - >>> your feedback is very welcomed! >>> >>> >>> >>> [1] https://lists.apache.org/thread/mz8hvz8dwhd0tzmv2lyobhlz7gtg4gq7 >>> <https://urldefense.com/v3/__https:/lists.apache.org/thread/mz8hvz8dwhd0tzmv2lyobhlz7gtg4gq7__;!!CiXD_PY!ThEMK5roxk1uO6Fpjmb3STnoNeOVFmjhytcQpHAT6WFXjpRioz7nJPSvMRRHwUXJovjHbRvmvA$> >>> >>> [2] >>> https://docs.google.com/document/d/1tKIyTk_-HhkmVuJsxvWP5eTELESpCBe_Vmb1nJ3Ia34/edit?usp=sharing >>> <https://urldefense.com/v3/__https:/docs.google.com/document/d/1tKIyTk_-HhkmVuJsxvWP5eTELESpCBe_Vmb1nJ3Ia34/edit?usp=sharing__;!!CiXD_PY!ThEMK5roxk1uO6Fpjmb3STnoNeOVFmjhytcQpHAT6WFXjpRioz7nJPSvMRRHwUXJoviZ8R6YKA$> >>> >>> >>> >>> — >>> >>> Alexey >>> >>> >>> >>> >>> >>> *As a recipient of an email from Talend, your contact personal data will >>> be on our systems. Please see our privacy notice. >>> <https://www.talend.com/privacy/>* >>> >>> >>> >> >>
