datacontract / datacontract-cli

CLI to manage your datacontract.yaml files
https://cli.datacontract.com
Other
484 stars 94 forks source link

Fix/311: Add enum and map type for avro schema #324

Closed aniketkapdule closed 4 months ago

aniketkapdule commented 4 months ago

Resolves issue #311

This PR adds enum and map type logic to the import avro_importer.py, resolving import errors.

simonharrer commented 4 months ago

There currently is no map data type in the spec. https://datacontract.com/#data-types

@jochenchrist three options:

  1. follow JSON Schema: type object with additionalProperties where one can define the types of the values
  2. follow Avro Schema: introduce a dedicated data type map with values field to define the substructures
  3. no map type at all

Not sure where we should be heading. Any thoughts?

jochenchrist commented 4 months ago

Created https://github.com/datacontract/datacontract-specification/issues/80 to discuss map

aniketkapdule commented 4 months ago

@jochenchrist Correct me if I am wrong, but I don't see any discussion about enum type. Should I raise a PR to also add enum in the FieldType?

jochenchrist commented 4 months ago

Regarding enum: I would prefer not to include enum as a logical data type in the specification. Such a data type is very rare. We have a property enum that we can use for this purpose, and we can set it accordingly during import and export.

jochenchrist commented 4 months ago

Like this:

      status:
        type: string
        required: true
        description: order status
        enum:
            - PLACED
            - SHIPPED
            - DELIVERED
            - CANCELLED
        config:
          avroType: enum
aniketkapdule commented 4 months ago

@jochenchrist I have made appropriate changes to accommodate enum type, as you have mentioned above, I will remove the changes from datacontract/lint/schema.py regarding map type once this issue is closed and pr is merged.

jochenchrist commented 4 months ago

Thank you for your contribution.

We added official support for Maps to the specification. 🎉 Also notice the new properties keys and values.

jochenchrist commented 4 months ago

I also updated the code, resolved merge conflicts, and removed the workaround in datacontract/lint/schema.py

aniketkapdule commented 4 months ago

I also updated the code, resolved merge conflicts, and removed the workaround in datacontract/lint/schema.py

Thank you @jochenchrist, Looking forward to do more contributions 👍