You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/05/22 00:54:24 UTC
[GitHub] [beam] damondouglas opened a new pull request, #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
damondouglas opened a new pull request, #17730:
URL: https://github.com/apache/beam/pull/17730
This PR is the first of four planned PRs. It contributes to [BEAM-14036](https://issues.apache.org/jira/browse/BEAM-14036) by implementing a configuration for reading from Pub/Sub.
I would like to request the following to review this PR.
R: @angoenka
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- ~Update `CHANGES.md` with noteworthy changes.~
- ~If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).~
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888439061
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
@chamikaramj (cc: @angoenka ) Thank you again for reviewing. May we consider leaving the name `getDeadLetterQueue`?
In a previous use of AutoValueSchema with AutoValue in a different project, I observed it needed the getters to be named as get<property name in camel case> in order for the serialization to work. For example, I had a property called fooName. When I named the getter fooName(), the return value of fooName() was null when invoked in the context of a DoFn. However, when I changed the getter to getFooName() the return value was what I expected. I am not sure if my observation is still valid.
Adding to supporting get instead of with, the design goals of the configuration class are to hold data needed by its corresponding SchemaProvider. The method is not doing any action implied by the with preposition. The getter is simply getting data.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888438246
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
May we consider leaving this as named? Please see https://github.com/apache/beam/pull/17730#discussion_r888167992 for further reasoning.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888166930
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
Review Comment:
I noticed when implementing the BigQuerySchemaTransform(Read/Write)Configuration, even though we have the nullable tags in the AutoValue class, the precommit checks failed. When the precommit gets resolved, I will see if removing this suppresswarnings annotation does not invoke a failure on the precommit checks.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888174272
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
@chamikaramj (cc: @angoenka )
I saw https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L128. Additionally, the original Provider's [https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L209](https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L209) calls the [config::serializer](https://github.com/apache/beam/blob/999bceab8e87d25f30faffe7d6431e2d8588663f/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L352) method. In said serializer method, the format, protoClass, and thriftClass attributes are referenced.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1155431017
I think we can just be compatible with existing SchemaIO config for now and update in the future if needed.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1133792581
Can one of the admins verify this patch?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1134868511
Run Java PreCommit
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1137528722
# [Codecov](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17730](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d9436c4) into [master](https://codecov.io/gh/apache/beam/commit/0c9cf43a7edae2e2a2622a8f4241b64a638121bb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c9cf43) will **decrease** coverage by `0.00%`.
> The diff coverage is `47.36%`.
> :exclamation: Current head d9436c4 differs from pull request most recent head 0c0ab4b. Consider uploading reports for the commit 0c0ab4b to get more accurate results
```diff
@@ Coverage Diff @@
## master #17730 +/- ##
==========================================
- Coverage 74.00% 74.00% -0.01%
==========================================
Files 696 695 -1
Lines 91851 91798 -53
==========================================
- Hits 67975 67931 -44
+ Misses 22627 22621 -6
+ Partials 1249 1246 -3
```
| Flag | Coverage Δ | |
|---|---|---|
| go | `50.44% <46.80%> (-0.02%)` | :arrow_down: |
| python | `83.73% <100.00%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [sdks/go/pkg/beam/core/runtime/exec/datasource.go](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9kYXRhc291cmNlLmdv) | `65.55% <0.00%> (+1.54%)` | :arrow_up: |
| [sdks/go/pkg/beam/pardo.go](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9wYXJkby5nbw==) | `47.41% <0.00%> (-3.03%)` | :arrow_down: |
| [sdks/python/apache\_beam/transforms/core.py](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdHJhbnNmb3Jtcy9jb3JlLnB5) | `92.30% <ø> (ø)` | |
| [sdks/go/pkg/beam/io/textio/textio.go](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9pby90ZXh0aW8vdGV4dGlvLmdv) | `55.15% <44.15%> (-10.42%)` | :arrow_down: |
| [sdks/go/pkg/beam/core/runtime/exec/sdf.go](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9nby9wa2cvYmVhbS9jb3JlL3J1bnRpbWUvZXhlYy9zZGYuZ28=) | `71.16% <76.92%> (+0.15%)` | :arrow_up: |
| [...n/apache\_beam/runners/dataflow/dataflow\_metrics.py](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9kYXRhZmxvdy9kYXRhZmxvd19tZXRyaWNzLnB5) | `74.83% <100.00%> (ø)` | |
| [sdks/python/apache\_beam/internal/gcp/auth.py](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vaW50ZXJuYWwvZ2NwL2F1dGgucHk=) | `73.33% <0.00%> (-5.34%)` | :arrow_down: |
| [.../python/apache\_beam/testing/test\_stream\_service.py](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdGVzdGluZy90ZXN0X3N0cmVhbV9zZXJ2aWNlLnB5) | `88.09% <0.00%> (-4.77%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/beam/pull/17730/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0c9cf43...0c0ab4b](https://codecov.io/gh/apache/beam/pull/17730?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r880840405
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
Review Comment:
Probably also add a "@Experimental" tag ?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
I don't see format, protoClass, thriftClass attributes in the original Read config.
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
Review Comment:
Please add correct nullable tags instead of supressing.
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Probably we should refer to PubSubIO.Read.withDeadLetterTopic() instead.
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
Pls change to getDeadLetterTopic to be consistent with the Pub/Sub read transform.
https://github.com/apache/beam/blob/d9436c41b3235a08bf8da693a562599b08039bf8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.java#L865
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
+
+ /** Used by the ProtoPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getProtoClass();
+
+ /**
+ * The subscription from which to read Pub/Sub messages.
+ *
+ * <p>See {@link PubsubIO.PubsubSubscription#fromPath(String)} for more details on the format of
+ * the subscription string.
+ */
+ @Nullable
+ public abstract String getSubscription();
+
+ /** Used by the ThriftPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getThriftClass();
+
+ /** Used by the ThriftPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getThriftProtocolFactoryClass();
+
+ /**
+ * When reading from Cloud Pub/Sub where record timestamps are provided as Pub/Sub message
+ * attributes, specifies the name of the attribute that contains the timestamp.
+ */
+ @Nullable
+ public abstract String getTimestampAttribute();
Review Comment:
Also add idAttribute ?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r894763538
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
> These again map to PubSubSchemaIO not the original PubSubIO. Is that plan here to make Pub/Sub SchemaTransform just consistent with existing Pub/Sub SchemaIO ? I'm not sure.
>
> @TheNeuralBit @dpcollins-google WDYT ? Should we just be consistent with Pub/Sub SchemaIO implementation here or should SchemaTransform be a different design ?
The plan question and replicate like for like SchemaIO are critical and blocking design decisions that relates to [this thread](https://github.com/apache/beam/pull/17730#discussion_r880850966) as well. I will hold off on any changes to this PR until we get the feedback needed. Thank you again.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . Is the recommendation here to change the name of the AutoValue getter as withDeadLetterTopic instead of getDeadLetterQueue? In a previous use of AutoValueSchema with AutoValue, I observed it needed the getters to be named as `get<property name in camel case>` in order to work. I am not sure if my observation is still valid.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . Is the recommendation here to change the name of the AutoValue getter as withDeadLetterTopic instead of getDeadLetterQueue?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888441418
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
@chamikaramj (cc: @angoenka ) May we consider keeping this as is? The [PubSubIO.Read.withDeadLetterTopic()](https://beam.apache.org/releases/javadoc/2.39.0/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.Read.html#withDeadLetterTopic-java.lang.String-) references `See PubsubIO.PubsubTopic.fromPath(String) for more details on the format of the deadLetterTopic string.` and provides no documentation on naming convention.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888179364
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
+
+ /** Used by the ProtoPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getProtoClass();
+
+ /**
+ * The subscription from which to read Pub/Sub messages.
+ *
+ * <p>See {@link PubsubIO.PubsubSubscription#fromPath(String)} for more details on the format of
+ * the subscription string.
+ */
+ @Nullable
+ public abstract String getSubscription();
+
+ /** Used by the ThriftPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getThriftClass();
+
+ /** Used by the ThriftPayloadSerializerProvider when serializing from a Pub/Sub message. */
+ @Nullable
+ public abstract String getThriftProtocolFactoryClass();
+
+ /**
+ * When reading from Cloud Pub/Sub where record timestamps are provided as Pub/Sub message
+ * attributes, specifies the name of the attribute that contains the timestamp.
+ */
+ @Nullable
+ public abstract String getTimestampAttribute();
Review Comment:
I will add this. Thank you!
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . In a previous use of AutoValueSchema with AutoValue in a different project, I observed it needed the getters to be named as `get<property name in camel case>` in order for the serialization to work. For example, I had a property called fooName. When I named the getter `fooName()`, the return value of `fooName()` was null when invoked in the context of a DoFn. However, when I changed the getter to `getFooName()` the return value was what I expected. I am not sure if my observation is still valid.
Adding to supporting `get` instead of `with`, the design goals of the configuration class are to hold data needed by its corresponding SchemaProvider. The method is not doing any action implied by the `with` preposition. The getter is simply getting data.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1155420191
@chamikaramj , @TheNeuralBit , @dpcollins-google , @pabloem If I don't see any activity on this PR in the next couple days, I will go ahead and close it to keep the PR space clean in this project.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888441418
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
May we consider keeping this as is? The [PubSubIO.Read.withDeadLetterTopic()](https://beam.apache.org/releases/javadoc/2.39.0/org/apache/beam/sdk/io/gcp/pubsub/PubsubIO.Read.html#withDeadLetterTopic-java.lang.String-) references `See PubsubIO.PubsubTopic.fromPath(String) for more details on the format of the deadLetterTopic string.` and provides no documentation on naming convention.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888223191
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
Review Comment:
Removed SuppressWarnings attribute and will watch future commit checks.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r894763538
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
> These again map to PubSubSchemaIO not the original PubSubIO. Is that plan here to make Pub/Sub SchemaTransform just consistent with existing Pub/Sub SchemaIO ? I'm not sure.
>
> @TheNeuralBit @dpcollins-google WDYT ? Should we just be consistent with Pub/Sub SchemaIO implementation here or should SchemaTransform be a different design ?
The plan and replicate like-for-like SchemaIO questions are critical and blocking design decisions that relates to [this thread](https://github.com/apache/beam/pull/17730#discussion_r880850966) as well. I will hold off on any changes to this PR until we get the feedback needed. Thank you again.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888166930
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
Review Comment:
I noticed when implementing the BigQuerySchemaTransform(Read/Write)Configuration, even though we have the nullable tags in the AutoValue class, the precommit checks failed. Unrelated but there is an overall problem with the Precommit checks for which we have a reported bug. When this larger precommit problem gets resolved, I will see if removing this suppresswarnings annotation does not invoke a failure on the precommit checks.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888222478
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
Review Comment:
Adding the Experimental annotation but we have an inconsistency that needs to be resolved with the BigQuerySchema configuration classes.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888170153
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
See https://github.com/apache/beam/pull/17730/files#r888167992
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r894108604
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
These again map to PubSubSchemaIO not the original PubSubIO. Is that plan here to make Pub/Sub SchemaTransform just consistent with existing Pub/Sub SchemaIO ? I'm not sure.
@TheNeuralBit @dpcollins-google WDYT ? Should we just be consistent with Pub/Sub SchemaIO implementation here or should SchemaTransform be a different design ?
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
I see, I think the confusion here is that this property does not correspond to that PubSubIO.getDeadLetterTopic I referenced by maps to the dlq property below.
https://github.com/apache/beam/blob/408664b3c3d7deccf22457f4338d2105576ae0c4/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubMessageToRow.java#L81
Is that correct ?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1133792582
Can one of the admins verify this patch?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17730:
URL: https://github.com/apache/beam/pull/17730#issuecomment-1133792580
Can one of the admins verify this patch?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888439061
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
@chamikaramj Thank you again for reviewing. May we consider leaving the name `getDeadLetterQueue`?
In a previous use of AutoValueSchema with AutoValue in a different project, I observed it needed the getters to be named as get<property name in camel case> in order for the serialization to work. For example, I had a property called fooName. When I named the getter fooName(), the return value of fooName() was null when invoked in the context of a DoFn. However, when I changed the getter to getFooName() the return value was what I expected. I am not sure if my observation is still valid.
Adding to supporting get instead of with, the design goals of the configuration class are to hold data needed by its corresponding SchemaProvider. The method is not doing any action implied by the with preposition. The getter is simply getting data.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . In a previous use of AutoValueSchema with AutoValue in a different project, I observed it needed the getters to be named as `get<property name in camel case>` in order for the serialization to work. For example, I had a property called fooName. When I named the getter `fooName()`, the return value of `fooName()` was null when invoked in the context of a DoFn. However, when I changed the getter to `getFooName()` the return value was what I expected. I am not sure if my observation is still valid.
Adding to supporting `get` instead of `with`, the design goals of the configuration class are to hold data needed by its corresponding SchemaProvider. The method is not doing any action implied by the `with` preposition. The getter is simply getting data.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888170153
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
Review Comment:
See https://github.com/apache/beam/pull/17730/files#r888167992
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888162856
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
Review Comment:
Thank you for reviewing!
@angoenka / @chamikaramj
We have the `@Experimental` tag on the BigQuerySchemaTransform(Read/Write)Provider but not the corresponding configurations. Should we add the experimental annotation on the corresponding configuration classes as well?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888174272
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
+ * letter queue topic string.
+ */
+ @Nullable
+ public abstract String getDeadLetterQueue();
+
+ /**
+ * The expected format of the Pub/Sub message.
+ *
+ * <p>Used to retrieve the {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializer} from
+ * {@link org.apache.beam.sdk.schemas.io.payloads.PayloadSerializers}.
+ */
+ @Nullable
+ public abstract String getFormat();
Review Comment:
I saw https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L128. Additionally, the original Provider's [https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L209](https://github.com/apache/beam/blob/master/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L209) calls the [config::serializer](https://github.com/apache/beam/blob/999bceab8e87d25f30faffe7d6431e2d8588663f/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaIOProvider.java#L352) method. In said serializer method, the format, protoClass, and thriftClass attributes are referenced.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . In a previous use of AutoValueSchema with AutoValue, I observed it needed the getters to be named as `get<property name in camel case>` in order to work. I am not sure if my observation is still valid.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] damondouglas commented on a diff in pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
damondouglas commented on code in PR #17730:
URL: https://github.com/apache/beam/pull/17730#discussion_r888167992
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/PubsubSchemaTransformReadConfiguration.java:
##########
@@ -0,0 +1,159 @@
+/*
+ * 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.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+package org.apache.beam.sdk.io.gcp.pubsub;
+
+import com.google.auto.value.AutoValue;
+import javax.annotation.Nullable;
+import org.apache.beam.sdk.schemas.AutoValueSchema;
+import org.apache.beam.sdk.schemas.Schema;
+import org.apache.beam.sdk.schemas.annotations.DefaultSchema;
+
+/**
+ * Configuration for reading from Pub/Sub.
+ *
+ * <p><b>Internal only:</b> This class is actively being worked on, and it will likely change. We
+ * provide no backwards compatibility guarantees, and it should not be implemented outside the Beam
+ * repository.
+ */
+@SuppressWarnings({
+ "nullness" // TODO(https://issues.apache.org/jira/browse/BEAM-10402)
+})
+@DefaultSchema(AutoValueSchema.class)
+@AutoValue
+public abstract class PubsubSchemaTransformReadConfiguration {
+
+ /** Instantiates a {@link PubsubSchemaTransformReadConfiguration.Builder}. */
+ public static Builder builder() {
+ return new AutoValue_PubsubSchemaTransformReadConfiguration.Builder();
+ }
+
+ /** The expected schema of the Pub/Sub message. */
+ public abstract Schema getDataSchema();
+
+ /**
+ * The Pub/Sub topic path to write failures.
+ *
+ * <p>See {@link PubsubIO.PubsubTopic#fromPath(String)} for more details on the format of the dead
Review Comment:
Thank you, @chamikaramj . In a previous use of AutoValueSchema with AutoValue in a different project, I observed it needed the getters to be named as `get<property name in camel case>` in order for the serialization to work. For example, I had a property called fooName. When I named the getter `fooName()`, the return value of `fooName()` was null when invoked in the context of a DoFn. However, when I changed the getter to `getFooName()` the return value was what I expected. I am not sure if my observation is still valid.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj merged pull request #17730: [BEAM-14036] Read Configuration for Pub/Sub SchemaTransform
Posted by GitBox <gi...@apache.org>.
chamikaramj merged PR #17730:
URL: https://github.com/apache/beam/pull/17730
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org