damccorm commented on code in PR #24670: URL: https://github.com/apache/beam/pull/24670#discussion_r1085405371
########## sdks/java/extensions/jackson/build.gradle: ########## @@ -7,7 +7,7 @@ * License); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.o Review Comment: ```suggestion * http://www.apache.org/licenses/LICENSE-2.0 ``` Seems like an accidental deletion ########## sdks/java/extensions/spd/README.md: ########## @@ -0,0 +1,67 @@ +# Structured Pipeline Descriptions (SPDs) Review Comment: Looks like there are some failing precommits ########## sdks/java/extensions/spd/build.gradle: ########## @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * License); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.o + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an AS IS BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +plugins { id 'org.apache.beam.module' } +applyJavaNature( + classesTriggerCheckerBugs: [ + 'StructuredPipelineDescription': 'https://github.com/typetools/checker-framework/issues/3792' Review Comment: Do we still need this? The bug was resolved 2.5 years ago and I think we've updated our checker version recently ########## sdks/java/extensions/python/src/main/java/org/apache/beam/sdk/extensions/python/transforms/DataframeTransform.java: ########## @@ -22,6 +22,7 @@ import org.apache.beam.sdk.util.PythonCallableSource; import org.apache.beam.sdk.values.PCollection; import org.apache.beam.sdk.values.Row; +import org.apache.beam.vendor.guava.v26_0_jre.com.google.common.collect.ImmutableList; Review Comment: Do we need this extra import? ########## sdks/java/extensions/spd/README.md: ########## @@ -0,0 +1,67 @@ +# Structured Pipeline Descriptions (SPDs) + +## Introduction + +Structured Pipeline Descriptions, henceforth SPDs (pronounced "Speedies") are a way of +modularizing and executing large Beam pipelines. It has been heavily inspired by and is +largely syntax-compatible with dbt Core and intends to play a similar role for data pipeline applications. + +SPD is emphatically **not** intended as a programming language and relies on Beam SDKs (SQL, Python, Java, Go) +to provide business logic implementation. It is aimed at structuring, testing and executing that business logic +in a pipeline, particularly Beam's multi-language pipelines. + +## Core Concepts + +As our aim is to make SPD familiar to people coming from dbt we share many of the same core concepts, +though modified for the data pipeline context. + + +### Models + +In SPD `models` represent a single named PCollection. Generally, this will be the expansion +of a composite transform in the underlying Beam transform. This is similar to the Beam SQL +`table` model, which serves as the underlying implementation for models in SPD. + +#### Supported "Languages" + +* **Beam SQL** is used to implement dbt-style SQL queries and we support a subset of the Jinja macro language that allows things like `ref` and `source` to work as expected. At the moment we are using the Calcite engine, but this could be configurable by the user. +* **Python** is implemented as an external transform calling to Beam's Python SDK. Like dbt you specify a function in a `.py` file that takes in a Dataframe as an argument. It doesn't actually matter what this function is named and unlike dbt the Dataframe argument comes from a `ref` or `source` macro included as a comment. +* **Typescript** Not yet implemented. Support here should be similar to Python. +* **Go** Not yet implemented and unclear what form it will take. Might end up being part of the "generic expansion service" model. +* **Javascript** (or other ScriptEngine languages). Not a real language/SDK but intended as a convient way of defining composite transforms and simple UDFs without resorting to writing a native Java PTransform just to do expansion. + +#### Materialization + +Like dbt, models can be materialized. At the moment we support `ephemeral` and `table` materialization +for models. The former is the default and means that the model only exists as a PCollection. When +materialized as a `table` data will egress via a WriteIO as defined in the current profile. For example, +if the output of SPD was defined to be, say, Pubsub a writeIO would be created. + +We have also considered a `view` materialization that would create the appropriate PCollectionView for use +as a side input, but it's not clear that this is necessary/desirable. + +### Sources + +Sources in SPD are essentially a special case of `model`, also representing a PCollection but always one supplied +by a ReadIO and configured from the pipeline's profile. Under the hood we are using Beam SQLs `TableProviders` to +provide read IO support. + +### Schemas + +One of the differences between dbt and SPD is that we often require schemas with data types (`columns:`) to be supplied. In many +situations this is not technically necessary as we could fetch the true schema from the underlying storage system (BigQuery, JDBC, etc). We have intentionally chosen not to do that to provide better support for CI/CD systems. + +One of the problems you often encounter with data pipelines are incompatible changes in upstream data sources. By specifying the expected schema within the SPD +pipeline we can validate that the external schema is still compatible with the expected schema of the pipeline itself, which should allow CI systems to implement +presubmit tests for schema migrations. + +## Profiles Review Comment: I'm having a bit of a hard time figuring out how to review this, and some of it is getting a grip on how the code is structured (some of that is unfamiliarity with the Java SDK, some of it is the size of the PR) and expected usage. I expect that is part of why it hasn't gotten traction with a review yet. Could you add a section to this Readme on how to use this (e.g. to run examples), and some info in the PR description or a comment on how the code is structured/any tips on how you would recommend approaching the review (e.g. start with X section, move to Y, finish with Z)? To this point, I've just been poking around and my comments aren't super substantive as a result I may also end up needing to defer to someone with a little more Java SDK background ########## spd/examples/profile.yml: ########## @@ -0,0 +1,19 @@ +# Example profile +near_earth_objects: + target: local + runners: + local: + runner: PortableRunner + jobEndpoint: "{{ env_var('LOCAL_RUNNER_HOST') }}:{{ env_var('LOCAL_RUNNER_PORT') }}" + defaultEnvironmentType: LOOPBACK + outputs: + local: + type: markdown + inputs: + local: + type: text + tables: # Let's you define external tables per profile (can be overridden in the project) + nasa_near_earth_objects: #TODO: Let you specify database/schema properly Review Comment: Could you create/associate an issue with this TODO? -- 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]
