dataops-tk / tap-powerbi-metadata

tap-powerbi-metadata
MIT License
5 stars 2 forks source link

Array of Complex Types throws error #10

Closed jtimeus-slalom closed 3 years ago

jtimeus-slalom commented 3 years ago

When trying to add an Array of Complex Types to my stream, I was getting the following error

TypeError: 'ComplexType' object is not callable

This is an example example record:

{"Id": "00000000-0000-0000-0000-000000000000", "Datasets": [{"DatasetId": "00000000-0000-0000-0000-000000000000", "DatasetName": "Dataset Final"}]}

below is the code that I tried in the PropertiesList

ArrayType("Datasets",ComplexType(
             StringType("DatasetId"),
             StringType("DatasetName")
            )
        ),
aaronsteers commented 3 years ago

After careful review and consideration, I found that this represents something of a weakness in how JSON schemas had been modeled, essentially conflating "properties" and "types". I've changed this implementation to more naturally distinguish between types and properties, with a thorough description here: https://gitlab.com/meltano/singer-sdk/-/merge_requests/8

After updating to the latest (via poetry add singer-sdk==0.0.2-dev.1068744002), please replace your sample code above with:

    Property(
        "Datasets",
        ArrayType(
            ObjectType(
                Property("DatasetId", StringType),
                Property("DatasetName", StringType),
            )
        )

The new convention (with formal Property mapping) is demonstrated in this sample code from the MR (now with an added example of how to implement the nested ArrayType(ObjectType):

    jsonschema = PropertiesList(
        Property("id", IntegerType, required=True),
        Property("name", StringType,
        Property("tags", ArrayType(StringType)),
        Property("ratio", NumberType),
        Property("days_active", IntegerType),
        Property("updated_on", DateTimeType),
        Property("is_deleted", BooleanType,
        Property(
            "author",
            ObjectType(
                Property("id", StringType),
                Property("name", StringType),
            )
        ),
        Property(
            "groups",
            ArrayType(
                ObjectType(
                    Property("id", StringType),
                    Property("name", StringType),
                )
            )
        ),

The old syntax, wherein type definitions have names and the required flag, should still work for a little longer, but eventually I think it will probably make sense to move entirely to the new Property syntax as shown above.

Also note that ComplexType (which expected a name argument) is being replaced by ObjectTypes (which does not accept a name argument).

Let me know if you have any difficulty implementing this spec or if you have any questions/concerns. I'm happy to lend a hand as needed.

jtimeus-slalom commented 3 years ago

I was able to solve this by converting to the new Property spec