[SPEC] Allow nested struct fields in SchemaDatasetFacet

[dolfinus]

Problem

Closes: #2051

Solution

• Your change modifies the core OpenLineage model
• Your change modifies one or more OpenLineage facets

One-line summary:

Allow nested fields support to SchemaDatasetFacet

Checklist

• You've signed-off your work
• Your pull request title follows our guidelines
• Your changes are accompanied by tests (if relevant)
• Your change contains a small diff and is self-contained
• You've updated any relevant documentation (if relevant)
• Your comment includes a one-liner for the changelog about the specific purpose of the change (if necessary)
• You've versioned the core OpenLineage model or facets according to SchemaVer (if relevant)
• You've added a header to source files (if relevant)

---

SPDX-License-Identifier: Apache-2.0
Copyright 2018-2023 contributors to the OpenLineage project

[dolfinus]

It looks like TypeResolver currently cannot handle recursive types

[mobuchowski]

@JDarDagran you had some ideas around nested types 🙂

[JDarDagran]

I did similar change to SchemaDatasetFacet. TypeResolver in Java for sure needs improvement to handle recursive types.

[dolfinus]

TypeResolver in Java for sure needs improvement to handle recursive types.

Any suggestions? My specialization is mostly Python, not Java.

[pawel-big-lebowski]

@JDarDagran @dolfinus It makes sense to support collection alike structures as well. I like the repeated keyword in protobuff. I think it would make sense to add repeated property in SchemaFieldItem with a default value equal to false.

[dolfinus]

@pawel-big-lebowski @JDarDagran Could we move this discussion to #2051?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (3a0d14c) to head (0c208dc).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2548   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          173       173           
=========================================
  Hits           173       173           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dolfinus]

Ok, TypeResolver now can handle self-referencing types.

Current implementation can break Java client backwards compatibility because method newSchemaDatasetFacetFieldsBuilder is renamed to newSchemaDatasetFacetFieldBuilder. Should I keep prior naming?

[JDarDagran]

@dolfinus that's fantastic you've made it to fix bug with recursive resolving in Java generator. I'm leaving review of this to @mobuchowski and @pawel-big-lebowski.

For me the change from SchemaDatasetFacetFields to SchemaDatasetFacetField is natural. More than that - I believe generator should change plural forms to singular in such cases (you can check my PR as an example)

[dolfinus]

I've removed repeated: boolean field from schema as a result of a discussion

[pawel-big-lebowski]

There's a test suite io.openlineage.client.OpenLineageTest which verifies how the generated OpenLineage object serialises to Json. Tests within that suite create programatically OL object, serialise & deserialise it, and then compare if those objects have same values.

Normally we don't such tests when modifying a facet. However, it makes sense now as new type is added to java poet generator. Additional advantage of this approach is that is shows how convenient it is to use the newly generated method (how to produce nested schema in java client)

[dolfinus]

I've added tests for serialization/deserialization to OpenLineageTest.factory. But IMHO it should be splitted to separated tests for each factory.