Adding Beam Schemas capability to parse json-schemas. This is the de-… by pabloem · Pull Request #24271 · apache/beam

pabloem · 2022-11-18T22:40:10Z

…facto standard way to define JSON schemas

fyi: @reuvenlax @lukecwik - I am adding a new dependency to Java Core SDK. It is not exposed in the API. It is used to parse JSON schemas.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

github-actions · 2022-11-18T23:35:13Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @apilloud for label java.
R: @damccorm for label build.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

codecov · 2022-11-18T23:44:47Z

Codecov Report

Merging #24271 (aff5ea4) into master (67e2008) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24271      +/-   ##
==========================================
- Coverage   73.43%   73.37%   -0.07%     
==========================================
  Files         716      717       +1     
  Lines       96571    96939     +368     
==========================================
+ Hits        70920    71128     +208     
- Misses      24328    24488     +160     
  Partials     1323     1323

Flag	Coverage Δ
python	`82.98% <ø> (-0.15%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...l/job_management/v1/beam_expansion_api_pb2_grpc.py	`51.85% <0.00%> (-6.05%)`	⬇️
.../python/apache_beam/testing/test_stream_service.py	`88.09% <0.00%> (-4.77%)`	⬇️
sdks/python/apache_beam/utils/interactive_utils.py	`95.12% <0.00%> (-2.44%)`	⬇️
...che_beam/runners/interactive/interactive_runner.py	`90.50% <0.00%> (-1.27%)`	⬇️
sdks/python/apache_beam/transforms/external.py	`78.88% <0.00%> (-0.86%)`	⬇️
sdks/python/apache_beam/pipeline.py	`92.12% <0.00%> (-0.15%)`	⬇️
sdks/python/apache_beam/dataframe/frames.py	`95.33% <0.00%> (-0.13%)`	⬇️
sdks/python/apache_beam/io/filebasedsource.py	`98.85% <0.00%> (-0.02%)`	⬇️
sdks/python/apache_beam/portability/common_urns.py	`100.00% <0.00%> (ø)`
...examples/inference/pytorch_image_classification.py	`0.00% <0.00%> (ø)`
... and 12 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

lukecwik

What do we do with properties that don't translate to beam schemas like array uniqueness of items?

We should describe to users what we reject and be explicit about what we support and what we ignore for each JSON type from https://json-schema.org/understanding-json-schema/reference/index.html

We'll also want to be careful when supporting nulls and test for the null case explicitly.

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

sdks/java/core/src/test/resources/schemas/json/nested_arrays_objects_json_schema.json

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

lukecwik · 2022-11-18T22:57:39Z

sdks/java/core/build.gradle

Should we make this feature require the user provide the library?

Need to analyze how large the dependency tree is and how stable it is.

I've marked is as a provided dependency.

Before moving forward - another possibility is to create a new extension module to hold these dependencies and this functionality. The module would be used by some of our schema transform implementations (kafka and pubsub), so it would still come back as a depenency for other Beam modules, but not for core - wdyt?

sdks/java/core/src/test/resources/schemas/json/nested_arrays_objects_json_schema.json

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

reuvenlax · 2022-11-19T21:06:26Z

Should we add a SchemaProvider as well?

damondouglas · 2022-11-21T22:27:10Z

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

Is it possible to add the following support methods? I advocate that removing the embedded checks in this method allows these to be independently tested. First, the following shows how beamSchemaFromJsonSchema might look with the proposed overloaded support methods. Then, some of the support methods follow. To avoid the instanceof checks, I tried to find in the libraries Javadoc, whether the base Schema had a getType() getter but it seems not. Perhaps we could have a Map<Class<T extends org.everit.json.schema.Schema>, Function<T extends org.everit.json.schema.Schema, Schema.Field>> that maps a particular json Schema class to the Java function that converts to the Schema.Field.

Schema.Builder beamSchemaBuilder = Schema.builder() for (String propertyName : jsonSchema.getPropertySchemas().keySet()) { org.everit.json.schema.Schema propertySchema = jsonSchema.getPropertySchemas().get(propertyName); if (propertySchema == null) { throw new IllegalArgumentException("Unable to parse schema " + jsonSchema.toString()); } Schema.Field field = beamFieldFromJsonField(propertySchema); builder = builder.addField(field); } return builder.build();

static Schema.Field beamFieldFromJsonField(org.everit.json.schema.Schema propertySchema) { if (propertySchema instanceof org.everit.json.schema.ObjectSchema) { return beamFieldFromJsonField((org.everit.json.schema.ObjectSchema) propertySchema); } if (propertySchema instanceof org.everit.json.schema.ArraySchema) { return beamFieldFromJsonField((org.everit.json.schema.ObjectSchema) propertySchema); } // etc etc to all the various types. } static Schema.Field beamFieldFromJsonField(org.everit.json.schema.ObjectSchema objectSchema) { // do some magic } static Schema.Field beamFieldFromJsonField(org.everit.json.schema.ArraySchema arraySchema) { // do some more magic } static Schema.Field beamFieldFromJsonField(org.everit.json.schema.BooleanField booleanSchema) { // do even some more magical magic }

The conversions for type to schema are short I think it would be simpler to not add this indirection

…facto standard way to define JSON schemas

pabloem · 2022-11-23T00:48:34Z

Should we add a SchemaProvider as well?

Can you elaborate @reuvenlax ? Any pointers? : ) Happy to implement whatever...

pabloem · 2022-11-23T03:12:32Z

Run Java PreCommit

pabloem · 2022-11-23T03:46:51Z

Run Java PreCommit

pabloem · 2022-11-23T17:37:19Z

Run Java PreCommit

pabloem · 2022-11-23T21:24:23Z

Run Java PreCommit

pabloem · 2022-11-24T07:21:06Z

What do we do with properties that don't translate to beam schemas like array uniqueness of items?

We should describe to users what we reject and be explicit about what we support and what we ignore for each JSON type from https://json-schema.org/understanding-json-schema/reference/index.html

We'll also want to be careful when supporting nulls and test for the null case explicitly.

I've tried to document in some detail what we do/don't support, as well as the required provided dependency. LMK what you think @lukecwik

I've also added support for nullable/non-nullable fields (based on the requiredfields tag)

pabloem · 2022-11-24T19:16:45Z

Run Spotless PreCommit

pabloem · 2022-11-24T19:16:51Z

Run Java_GCP_IO_Direct PreCommit

pabloem · 2022-11-24T19:17:00Z

Run Java_GCP_IO_Direct PreCommit

pabloem · 2022-11-28T17:39:14Z

Run Java_Debezium_IO_Direct PreCommit

pabloem · 2022-11-28T17:39:21Z

Run Java_PVR_Flink_Docker PreCommit

pabloem · 2022-11-28T17:39:43Z

@lukecwik please let me know what you think of the javadoc for it, and other changes.

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

lukecwik · 2022-12-01T22:04:42Z

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

+ *  <dependency>
+ * 	  <groupId>com.github.erosb < /groupId>
+ * 	  <artifactId>everit-json-schema < /artifactId>
+ * 	  <version>1.14.1 < /version>


We should point the person to use the same version that Beam was tested with and not hard-code the version in the documentation.

Done. Thanks.

annoying but is fine

sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java

lukecwik · 2022-12-01T22:36:05Z

Should we add a SchemaProvider as well?

Can you elaborate @reuvenlax ? Any pointers? : ) Happy to implement whatever...

This only applies if you have a type that you want to convert to a schema and doesn't apply to schemas stored in an arbitrary location (e.g. string, file, URL, ...)

lukecwik · 2022-12-01T22:36:55Z

@lukecwik please let me know what you think of the javadoc for it, and other changes.

Nice improvement over the original and good tests.

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

pabloem · 2022-12-02T20:41:10Z

Run Java_Examples_Dataflow PreCommit

pabloem · 2022-12-02T20:42:52Z

Run Java_Examples_Dataflow PreCommit

pabloem · 2022-12-02T20:59:11Z

Run Java_Examples_Dataflow PreCommit

pabloem · 2022-12-02T21:05:21Z

Run Java_Examples_Dataflow PreCommit

pabloem · 2022-12-02T21:10:02Z

Run Java_GCP_IO_Direct PreCommit

pabloem · 2022-12-02T21:47:02Z

thanks all!

apache#24271) * Adding Beam Schemas capability to parse json-schemas. This is the de-facto standard way to define JSON schemas * json sample schema files for tests * addressing comments * fixup * fixup * documenting and fixing nullable cases * fixup * Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com> * Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com> * Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com> * Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com> * Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com> * improve docs * fixup Co-authored-by: Lukasz Cwik <lcwik@google.com>

github-actions bot added build java labels Nov 18, 2022

pabloem mentioned this pull request Nov 18, 2022

Support for JsonSchema in Kafka Read Schema Transform #24272

Merged

4 tasks

github-actions bot added the Next Action: Reviewers label Nov 18, 2022

lukecwik requested changes Nov 19, 2022

View reviewed changes

pabloem mentioned this pull request Nov 19, 2022

Adding support for PubSub Lite in Schema Transforms #24275

Merged

4 tasks

damondouglas reviewed Nov 21, 2022

View reviewed changes

pabloem added 3 commits November 22, 2022 13:37

Adding Beam Schemas capability to parse json-schemas. This is the de-…

6add8d0

…facto standard way to define JSON schemas

json sample schema files for tests

2d77e7a

addressing comments

a14c8c4

pabloem force-pushed the jsonschema-parse branch from 35131d3 to a14c8c4 Compare November 22, 2022 21:37

fixup

342edca

pabloem force-pushed the jsonschema-parse branch from 953e918 to 342edca Compare November 23, 2022 00:01

fixup

0162d05

pabloem added 2 commits November 23, 2022 19:33

documenting and fixing nullable cases

f0a029d

fixup

88c9176

pabloem requested a review from lukecwik November 29, 2022 20:49

lukecwik approved these changes Dec 1, 2022

View reviewed changes

pabloem and others added 7 commits December 2, 2022 13:46

Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils…

bcb19dc

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils…

1fe9e77

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils…

d7c787a

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils…

426c48b

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

Update sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils…

7560af0

…/JsonUtils.java Co-authored-by: Lukasz Cwik <lcwik@google.com>

improve docs

254c258

fixup

aff5ea4

pabloem force-pushed the jsonschema-parse branch from 45cfdd1 to aff5ea4 Compare December 2, 2022 19:19

pabloem merged commit 2dbf94f into apache:master Dec 2, 2022

pabloem deleted the jsonschema-parse branch December 2, 2022 21:46

mosche mentioned this pull request Dec 15, 2022

[Bug]: POM of beam-sdks-java-core 2.45.0-SNAPSHOT is broken #24675

Closed

15 tasks

mosche mentioned this pull request Dec 22, 2022

Fix POM of beam-sdks-java-core #24760

Merged

3 tasks

Conversation

pabloem commented Nov 18, 2022

GitHub Actions Tests Status (on master branch)

Uh oh!

github-actions bot commented Nov 18, 2022

Uh oh!

codecov bot commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lukecwik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukecwik Nov 18, 2022

Choose a reason for hiding this comment

Uh oh!

pabloem Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

reuvenlax commented Nov 19, 2022

Uh oh!

damondouglas Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukecwik Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

pabloem commented Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pabloem commented Nov 23, 2022

Uh oh!

pabloem commented Nov 23, 2022

Uh oh!

pabloem commented Nov 23, 2022

Uh oh!

pabloem commented Nov 23, 2022

Uh oh!

pabloem commented Nov 24, 2022

Uh oh!

pabloem commented Nov 24, 2022

Uh oh!

pabloem commented Nov 24, 2022

Uh oh!

pabloem commented Nov 24, 2022

Uh oh!

pabloem commented Nov 28, 2022

Uh oh!

pabloem commented Nov 28, 2022

Uh oh!

pabloem commented Nov 28, 2022

Uh oh!

Uh oh!

lukecwik Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

pabloem Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

lukecwik Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukecwik commented Dec 1, 2022

Uh oh!

lukecwik commented Dec 1, 2022

codecov bot commented Nov 18, 2022 •

edited

Loading

damondouglas Nov 21, 2022 •

edited

Loading

pabloem commented Nov 23, 2022 •

edited

Loading