+1 On 7/27/21 7:55 PM, Chamikara Jayalath wrote:
On Tue, Jul 27, 2021 at 10:51 AM Jan Lukavský <je...@seznam.cz <mailto:je...@seznam.cz>> wrote:I agree that adding PipelineOptions is technically (nearly) orthogonal, it is not exactly independent, because the proposal emphasize the need for it. On the other hand, given the complexity of the original proposal, I think that this change is of really low complexity, it might boil down to adding a "repeated string options" field to the ExpansionRequest (or a map, or whatever), and then reading it back during the expansion. I think that taking the opportunity of making a somewhat larger change will be better in this case, because it will instantly enable a broader usage. But - if we want to treat it independly, can we create a tracking Jira for that?We have a tracking JIra for the PipelineOptions part: https://issues.apache.org/jira/browse/BEAM-9449 <https://issues.apache.org/jira/browse/BEAM-9449>Jan On 7/27/21 7:09 PM, Chamikara Jayalath wrote:On Tue, Jul 27, 2021 at 12:18 AM Jan Lukavský <je...@seznam.cz <mailto:je...@seznam.cz>> wrote: Hi Cham, I think this approach is great, but what I'm missing is PipelineOptions for the expansion. This approach makes it possible (and practical) to use single expansion service for many different Pipelines and Environments - I can imagine a "single" (logical) instance of the expansion service per data team/organisation (packaging the expansion service with possibly many dependencies might be non-trivial), that means that the expansion might need customization that is described precisely by PipelineOptions that are passed to the Pipeline that is used for the expansion. Other than that this looks great. Good point. I think that sending in the PipelineOptions as a part of the expansion service is an orthogonal change that can be useful for both config object based and class/type lookup based expansions. So I'd like to consider that separately instead of tying into this work. Thanks, Cham Jan On 7/27/21 6:16 AM, Chamikara Jayalath wrote:On Mon, Jul 26, 2021 at 8:26 PM Robert Burke <rob...@frantil.com <mailto:rob...@frantil.com>> wrote: Looked at it. LGTM It looks portable enough to be useful for any future Go SDK based Expansion services. I do wonder if there are more general names than "class" but that's a terminology quibble anyway. (Go doesn't use that term, as Go doesn't have inheritance based polymorphism.) Perhaps "type" is a good replacement, which is in use in most languages in one capacity or another? Yeah, probably we can call it "type" and describe what the field means for each SDK in a comment. Certainly not a hard blocker. On Mon, Jul 26, 2021, 7:10 PM Chamikara Jayalath <chamik...@google.com <mailto:chamik...@google.com>> wrote: Hi All, Currently, to define Java cross-language transforms, users have to define three new Java classes: a Registrar, a Builder and a Config Object [1]. While this might not be too hard for a Java programmer, learning Java and developing/building/releasing new classes just to use existing Java transforms may be cumbersome for Python and Go users. To further simplify the process for defining new Java cross-language transforms and usage of such transforms from other SDKs I would like to propose an update to the cross-language transform expansion protocol. Please see the following for details and let me know if you have any comments. https://docs.google.com/document/d/1ECXSWicE31K-vSxdb4qL6UcmovOAWvE-ZHFT3NTM654/edit?usp=sharing <https://docs.google.com/document/d/1ECXSWicE31K-vSxdb4qL6UcmovOAWvE-ZHFT3NTM654/edit?usp=sharing> Thanks, Cham [1] https://beam.apache.org/documentation/programming-guide/#create-x-lang-transforms <https://beam.apache.org/documentation/programming-guide/#create-x-lang-transforms>
