> On 13 May 2022, at 18:38, Robert Bradshaw <rober...@google.com> wrote: > > On Fri, May 13, 2022 at 5:56 AM Moritz Mack <mm...@talend.com > <mailto:mm...@talend.com>> wrote: >> >> Thanks so much for all these pointers, Alexey. Having that context really >> helps! >> >> Skimming through the past conversations, this one key consideration hasn’t >> changed and seems still critical: >> >> AvroCoder is the de facto standard for encoding complex user types (with >> SchemaCoder still being experimental). > > We should probably remove the experimental annotations from > SchemaCoder at this point.
Is there anything that stops us from that? > >> Consequently, any backwards incompatible change in that respect would >> massively impact users. > > True. Some changes would be more painful than others though. I think that this is a main point of the difficulties with some progress on this topic - make this as smooth as possible for users. In the same time, keep the dependencies with several CVEs for a long time is not good thing as well. > >> Having that in mind, here’s my 2 cents on the options mentioned: >> >> Bump Avro in core: >> >> This will seamlessly work for users that just need a coder for complex >> types, but don’t use Avro themselves. >> Anyone using Avro APIs from user code might get into trouble. >> >> Support different Avro versions & making the dependency provided >> >> In terms of user impact, making Avro provided will be very troublesome. >> Unless Avro remains available on the classpath through different ways, this >> would break things in a really bad way. Worst of all, it might not even >> break at compile time but at runtime only. >> I don’t think this option requires Avro being a provided dependency. If the >> default is kept as is, anyone can (safely) bump Avro to a higher (supported) >> version on their classpath. In that case it would be good to warn users >> about CVEs in logs if they remain on an old version of Avro. But I’m not >> sure how feasible (reliable) it is to detect the Avro minor version given >> that things might be repackaged into fat jars. > > This seems both difficult and potentially dangerous. What do you mean by “potentially dangerous”, Robert? Any specific point mentioned above or just having another version of Avro jars in the classpath? > >> Extract Avro as an extension >> >> I agree with Brian, I don’t think there’s a need to keep a shaded/vendored >> Avro in core… but > > If we can leave it out, that'd be great. > >> It’s pretty hard to get this done without causing trouble for users. Making >> this a seamless migration would require core to depend on the new extension >> module and to maintain all the old public user facing APIs. > > I think a change that breaks loudly at compile time (if the features > in question are being depended on), with a clear fix (e.g. "depend on > this instead") is the "cleanest" way to do a breaking change. > > On this note, would it be possible to do explicit "without avro" and > "with avro" targets/variants in the core module, with plain old "core" > being an alias first to the latter, and then to the former, according > to some deprecation timeline? (I'm not familiar enough with > gradle/maven to know if this is possible.) This could unblock users > needing a new avro in the short run while providing a longer-term > migration path. (Ideally the (relatively limited) avro parts of core > would also get moved to their own extension under their own namespace, > and the core one would remain solely for backwards compatibility > reasons.) Good point. Are you talking about keeping the default “core" artifacts with old Avro version (as for now) as a main dependency and provide additionally other “core” artifacts with a recent Avro version or even as provided for users that need other Avro versions? > >> Duplicating packages between core and the new extension module is going to >> cause issues with Java 11. That pretty much means existing public user >> facing APIs have to remain as a (deprecated) façade in core using the code >> in the new extension. >> The above makes the effort almost look worthless in the short term, but it >> would certainly help to have a well-defined scope that can be tested for >> compatibility with multiple different Avro versions. >> >> >> >> To finally make progress on this topic, I’d suggest starting off with >> supporting multiple different Avro versions in core (2) but to keep the >> default Avro version as is. >> >> At the same time, extracting Avro as extension in a backwards compatible way >> seems to be the right path forward long term together with a well >> communicated deprecation plan. >> >> >> >> Best, >> >> Moritz >> >> >> >> >> >> From: Brian Hulette <bhule...@google.com> >> Date: Thursday, 12. May 2022 at 23:21 >> To: dev <dev@beam.apache.org> >> Subject: Re: [DISCUSS] Next steps for update of Avro dependency in Beam >> >> Regarding Option (3) "but keep and shade Avro for “core” needs as v.1.8.2 >> (still have an issue with CVEs)" Do we actually need to keep avro in core >> for any reason? I thought we only had it in core for AvroCoder, schema >> support, and >> >> ZjQcmQRYFpfptBannerStart >> >> This Message Is From an External Sender >> >> This message came from outside your organization. >> >> Exercise caution when opening attachments or clicking any links. >> >> ZjQcmQRYFpfptBannerEnd >> >> Regarding Option (3) "but keep and shade Avro for “core” needs as v.1.8.2 >> (still have an issue with CVEs)" >> >> >> >> Do we actually need to keep avro in core for any reason? I thought we only >> had it in core for AvroCoder, schema support, and IOs, which I think are all >> reasonable to separate out into an extension (this would be comparable to >> the protobuf extension). To confirm I just grepped for files in core that >> import avro: >> >> ❯ grep -liIrn 'import org\.apache\.avro' sdks/java/core/src/main >> sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroByteBuddyUtils.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ConvertHelpers.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroUtils.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/AvroRecordSchema.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/DynamicAvroDestinations.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/SerializableAvroCodecFactory.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroSink.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroSource.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroSchemaIOProvider.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/AvroIO.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/io/ConstantAvroDestination.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroGenericCoder.java >> sdks/java/core/src/main/java/org/apache/beam/sdk/coders/AvroCoder.java >> >> Brian >> >> >> >> On Thu, May 12, 2022 at 2:08 PM Robert Bradshaw <rober...@google.com> wrote: >> >> Keeping avro in our public (core) API and as an internal dependency >> seems to be a recurring pain point, I would be all for pulling it out >> (basically option 3) and subsequently updating our internal version >> (hopefully no backwards compatibility issues here) and letting the >> extension live with a variety of versions (insofar as this is >> feasible). >> >> On Thu, May 12, 2022 at 10:29 AM Alexey Romanenko >> <aromanenko....@gmail.com> wrote: >>> >>> Hi everyone, >>> >>> Sorry in advance for a long email. >>> TL;DR: Let’s discuss the next steps to update Avro dependency in Beam. >>> >>> I’d like to come back to this old and quite sensitive topic here which is >>> Apache Avro version update in Beam. Along the time, we already had several >>> discussions on this (for example [1]) but without any concrete resolutions >>> in the end, iirc. >>> >>> As we all know, Beam still depends on quite old Avro version 1.8.2 and >>> there were some attempts to bump it to more recent ones. One of the main >>> reasons to bump an Avro version, imho, is that Avro 1.8.2 dependency brings >>> several CVEs [2], but the latest Avro 1.11.0 brings only one [3] >>> >>> In the same time, this update with introduce some incompatible changes that >>> Avro has between versions and this may affect Beam users and potentially it >>> may affect transitive dependencies while using Beam with other project that >>> use Avro as well: >>> - Avro completely moved to java.time.* instead of org.joda.time.*. So, we >>> need to adjust date/time conversions from/to Beam schema accordingly since >>> Beam schema still uses joda.time. It will require users to regenerate >>> already generated Java code with avro-compiler (if any) otherwise it won’t >>> compile; >>> - Some minor changes in Avro dependencies and user API; >>> - Something else? >>> >>> I know that here, on the list, we have people from Avro community that are >>> much more experienced in this than me - so, please correct me if I say >>> something wrong or not 100% correct. >>> >>> >>> In Beam, we also performed several attempts to update Avro - for example, >>> [4], [5], [6] and others. >>> >>> To make such update easier in the future, we also discussed to move Avro >>> dependency out of core Beam [7] and there were an attempt to do that [8] by >>> finally this PR was closed with a resolution that it’s not actually needed >>> and we may just want to test Beam with different Avro versions [9] >>> >>> The latest work on this was a PR to support several versions of Avro in >>> Beam (1.8.x and 1.9.x) [10] which still introduces some breaking changes >>> for users, iirc. >>> >>> So, seems that we are a bit stuck on this topic, though, imho, we need to >>> decide how move forward mostly because of CVEs in old Avro versions and >>> future Avro updates in Beam. >>> >>> The potential options (as I can see them): >>> >>> 1) Bump Avro dependency to the latest one (1.11.0) or the possible more >>> recent one >>> - Pros: >>> - latest/recent Avro dependency; >>> - potentially easy to update in the future; >>> - Cons: >>> - breaking change for users; >>> - potentially issues with other projects that use Avro (like Apache Spark >>> e.g.). >>> >>> 2) Support different Avro versions in Beam, make Avro dependency provided >>> - Pros: >>> - user decides which versions to use; >>> - easy to update in the future; >>> - Cons: >>> - breaking change for users; >>> - not fact that it’s possible to implement in reality; >>> - more tests to test Beam with different Avro versions >>> >>> 3) Extract Avro as an extension, like we do for other formats, and update >>> to latest Avro version, but keep and shade Avro for “core” needs as v.1.8.2 >>> (still have an issue with CVEs) >>> >>> 4) Anything else? >>> >>> >>> Please, share your thoughts on this and correct me if I stated something >>> wrong. The goal of this discussion is finally to move forward with Avro >>> update topic. >>> >>> — >>> Alexey >>> >>> >>> [1] https://lists.apache.org/thread/bkwrbqg2nwp1xq1j57xt3kvmy93vpj9r >>> [2] https://mvnrepository.com/artifact/org.apache.avro/avro/1.8.2 >>> [3] https://mvnrepository.com/artifact/org.apache.avro/avro/1.11.0 >>> [4] https://github.com/apache/beam/pull/9779 >>> [5] https://github.com/apache/beam/pull/17372 >>> [6] https://github.com/apache/beam/pull/17246 >>> [7] https://lists.apache.org/thread/fw4w6xgm05nl5cg502co97pt6cygt4on >>> [8] https://github.com/apache/beam/pull/12748 >>> [9] https://lists.apache.org/thread/y76wjqprm8dyfxxfwcqbzxtht2qkrgzg >>> [10] https://github.com/apache/beam/pull/16271 >>> >>> >>> >>> >>> >>> >> >> As a recipient of an email from Talend, your contact personal data will be >> on our systems. Please see our privacy notice.
