Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

aniketkapdule
Copy link
Contributor

@aniketkapdule aniketkapdule commented Jul 12, 2024

  • Tests pass
  • ruff format
  •  CHANGELOG.md entry added

Resolves issue #311

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

@aniketkapdule aniketkapdule marked this pull request as ready for review July 12, 2024 12:42
@simonharrer
Copy link
Contributor

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
Copy link
Contributor

Created datacontract/datacontract-specification#80 to discuss map

@aniketkapdule
Copy link
Contributor Author

aniketkapdule commented Jul 14, 2024

@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
Copy link
Contributor

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
Copy link
Contributor

Like this:

      status:
        type: string
        required: true
        description: order status
        enum:
            - PLACED
            - SHIPPED
            - DELIVERED
            - CANCELLED
        config:
          avroType: enum

@aniketkapdule aniketkapdule force-pushed the fix-add-enum-and-map-type-for-avro-schema branch from 5ee440e to 4104c19 Compare July 15, 2024 10:45
@aniketkapdule aniketkapdule force-pushed the fix-add-enum-and-map-type-for-avro-schema branch from 4104c19 to 5384d08 Compare July 15, 2024 10:49
@aniketkapdule aniketkapdule force-pushed the fix-add-enum-and-map-type-for-avro-schema branch from 5384d08 to 2a146c5 Compare July 15, 2024 10:52
@aniketkapdule
Copy link
Contributor Author

@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.

…nd-map-type-for-avro-schema

# Conflicts:
#	CHANGELOG.md
#	tests/fixtures/avro/data/orders.avsc
@jochenchrist
Copy link
Contributor

Thank you for your contribution.

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

@jochenchrist
Copy link
Contributor

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

@jochenchrist jochenchrist merged commit f817893 into datacontract:main Jul 18, 2024
4 checks passed
@aniketkapdule
Copy link
Contributor Author

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants