avro icon indicating copy to clipboard operation
avro copied to clipboard

AVRO-1329: Extended enums

Open unchuckable opened this issue 5 years ago • 1 comments

Sorry this took a little. Corona and RL made me have to deal with other issues before this one. Feedback welcome. I tried to provide the functionality of AVRO-1329 along with some suitable yet compatible changes to the public API, but am well willing to make changes as deemed appropriate.

Thanks, and stay safe.

---8<------

Make sure you have checked all steps below.

Jira

  • [X] My PR addresses the following Avro Jira issues and references them in the PR title. For example, "AVRO-1234: My Avro PR"
    • https://issues.apache.org/jira/browse/AVRO-1329

Tests

  • [X] My PR adds the following unit tests:
  • TestSchema.testExtendedEnumSchemaSerializeAndDeserialize
  • TestSchema.testSimpleEnumSchemaSerialize
  • Updates/Extensions to Compiler Unit tests

Commits

  • [X] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • [X] In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

unchuckable avatar Jan 22 '21 14:01 unchuckable

Dunno what these CI issues are, can anyone help there?

unchuckable avatar Jan 22 '21 22:01 unchuckable

@Humbedooh - Hi, Daniel. Can you please elaborate as to why this ticket is being closed?

unchuckable avatar Oct 06 '23 08:10 unchuckable

@unchuckable, I guess it's because this pull request targeted the "master" branch, which has now been renamed to "main" (AVRO-3878, https://github.com/apache/avro/pull/2537). Are you able to retarget this to "main"?

I would like to have "symbol-props" or similar at least added to the Avro specification, even if the per-language libraries were not changed to recognize it. I have a custom application that generates HTML documentation from Avro schemas, and I'd be able to implement "symbol-props" there and then migrate the per-symbol documentation from the "doc" properties of my enum schemas.

KalleOlaviNiemitalo avatar Oct 06 '23 09:10 KalleOlaviNiemitalo

https://issues.apache.org/jira/browse/INFRA-25047

martin-g avatar Oct 06 '23 09:10 martin-g

@KalleOlaviNiemitalo, @martin-g Thanks for the explanation. Makes sense. Nice to see not only us have a use case for this. Basically, what I would like to add at a later point, would be enum symbol aliases similar to field aliases, that would help us a lot in schema evolution. I think I'll try to rebase this to current versions and re-submit a pull request with the new main branch.

unchuckable avatar Oct 06 '23 09:10 unchuckable

Hovering over the Reopen and comment button below shows this hint: The repository that submitted this pull request has been deleted :-/

martin-g avatar Oct 06 '23 11:10 martin-g

Interesting. Something must have been screwed up there. Will recreate the change to the best of my ability ;)

unchuckable avatar Oct 06 '23 12:10 unchuckable

Found the original code again, will rebase onto current main and resubmit the pull request these days.

unchuckable avatar Oct 06 '23 12:10 unchuckable

I tried implementing "symbol-props" in my doc generator application. It is a bit easier with the map style

{
    "type": "enum",
    "symbols": ["zero", "one", "two"],
    "symbol-props": {
        "zero": {},
        "one": { "doc": "uno" }
    }
}

than with the array style

{
    "type": "enum",
    "symbols": ["zero", "one", "two"],
    "symbol-props": [
        { "name": "zero" },
        { "name": "one", "doc": "uno" }
    ]
}

because my application can loop over the value of "symbols" and use a JSON DOM method to easily look up the corresponding entry from the "symbol-props" map. In the array style, the symbol names are in the "name" properties, and my application has to construct a dictionary from those before it starts looping over "symbols".

But perhaps consistency with record schemas is more important than convenience here.

KalleOlaviNiemitalo avatar Oct 06 '23 13:10 KalleOlaviNiemitalo

Found the original code again, will rebase onto current main and resubmit the pull request these days.

If you had not found the original code, you could still have fetched it from GitHub, e.g. by running git fetch https://github.com/apache/avro/ refs/pull/1067/head. That ref is listed by git ls-remote.

KalleOlaviNiemitalo avatar Oct 07 '23 03:10 KalleOlaviNiemitalo

Please rename "symbol-props" to "symbolProps", for consistency with the "logicalType" attribute. In the current Avro specification, the kebab-case style is used only for names of logical types, e.g. "time-millis".

KalleOlaviNiemitalo avatar Oct 07 '23 06:10 KalleOlaviNiemitalo