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

add AvroUnionPosition attributute to control position of elements in unions #587

Merged
merged 2 commits into from Nov 22, 2020

Conversation

davideicardi
Copy link
Contributor

@davideicardi davideicardi commented Nov 15, 2020

AvroSortPriority takes a priority to sort the elements of the union, but it is quite strange from the user perspective because the main goal is to set the elements inside the union in the order that we expect.

I have written a @AvroUnionPosition attribute that works in the same way but takes a position. It will be used like:

  sealed trait Fruit
  @AvroUnionPosition(0)
  case object Unknown extends Fruit
  @AvroUnionPosition(1)
  case class Orange(size: Int) extends Fruit
  @AvroUnionPosition(2)
  case class Mango(size: Int) extends Fruit

And it will produce an union with [Unknown, Orange, Mango]. I think it is more clear from the user perspective.

Also when we want to ensure backward/forward compatibility this can allow us to just add new elements without touching the existing ones. For example I can add an Apple with position 3.

@davideicardi davideicardi changed the title add AvroUnionPosition attributute to control position of elements in … add AvroUnionPosition attributute to control position of elements in unions Nov 15, 2020
@davideicardi davideicardi force-pushed the feature/avro-union-position branch 2 times, most recently from fb6d077 to ae8f28d Compare November 15, 2020 14:09
@davideicardi davideicardi marked this pull request as ready for review November 15, 2020 14:23
@sksamuel
Copy link
Owner

Looks great. Can we add a test that if an element is not annotated it appears at the end.

@davideicardi
Copy link
Contributor Author

@sksamuel Added test for not annotated elements.
thanks!

@davideicardi
Copy link
Contributor Author

davideicardi commented Nov 21, 2020

I have also updated setup-scala github action to v10 to fix this security issue: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

Otherwise the PR's workflow generates an exception, see https://github.com/sksamuel/avro4s/runs/1435858868

@sksamuel sksamuel merged commit b82a60d into sksamuel:master Nov 22, 2020
@sksamuel
Copy link
Owner

Great work thanks.

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

2 participants