Yes, the schema is build at Pipeline construction time, I'm pushing all of the *complexity to the To/FromRow* functions. They will have an overlay of the fields that convert to and from Proto. Work is moving alone file for Proto support, working now: - Primitives - Wrapper types - Timestamp
Everything will be ready for the Beam Summit :-) _/ _/ Alex Van Boxel On Wed, May 15, 2019 at 3:26 AM Reuven Lax <[email protected]> wrote: > > > *From: *Alex Van Boxel <[email protected]> > *Date: *Tue, May 14, 2019 at 3:38 PM > *To: *ML Beam/Dev > > ProtoBuf and certainly the Descriptor is a challenging beast, and I >> certainly want to support DynamicMessage (see also my ProtoCoder PR). >> >> Creating a schema from the proto is easy, the trick is creating the >> to/fromRow. With precomiled proto's I can easily get the Descriptor from >> the class, but the is not available for DynamicMessages. I need the >> descriptor to get the fields (via the FieldDescriptor). >> > > However you do have the schema at graph-construction time, don't you? > > >> >> If I can't store it in the schema, I will probably need to store the >> Descriptor in the Serializable toRow/fromRow: it's here that the >> FieldDescriptor is required. >> > > Yes, that should be fine. > > >> >> I had my doubts that Schema was allowed to be extensible... I'll add a PR >> to make Schema final, should be a no brainer. >> _/ >> _/ Alex Van Boxel >> >> >> On Tue, May 14, 2019 at 8:05 PM Reuven Lax <[email protected]> wrote: >> >>> Can you explain what you're trying to do? I don't think that embedding >>> the proto descriptor in the schema is a great way to go, but I may not be >>> understanding the use case. >>> >>> *From: *Alex Van Boxel <[email protected]> >>> *Date: *Tue, May 14, 2019 at 9:00 AM >>> *To: *ML Beam/Dev >>> >>> Hi Schema lovers, >>>> >>>> I'm implementing schema support for Protobuf and I was wondering if >>>> it's allowed to override Schema. It looks tempting (as it's not final), as >>>> I need a container for the Proto Descriptor. >>>> >>>> For normal pre-compiled classes it's not required, but for >>>> DynamicMessage it is. If I would be able to store it in Schema I can reuse >>>> it in to/fromRow. >>>> >>>> Thoughts? >>>> >>>> _/ >>>> _/ Alex Van Boxel >>>> >>>
