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

Modularization: avro module #115

Open
fabiojose opened this issue Apr 15, 2020 · 14 comments · May be fixed by #407
Open

Modularization: avro module #115

fabiojose opened this issue Apr 15, 2020 · 14 comments · May be fixed by #407
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@fabiojose
Copy link
Contributor

This module implements the Avro Event Format.

  • avro

This module depend on api one.

When someone wants the avro format to create or read events, they add this module and they concern about potential conflicts with pre-existing dependencies in their projects.

@slinkydeveloper slinkydeveloper added the enhancement New feature or request label Apr 21, 2020
@slinkydeveloper slinkydeveloper added the help wanted Extra attention is needed label Jun 15, 2020
@subanasif
Copy link

subanasif commented Dec 11, 2020

@slinkydeveloper: I do have implementation of EventFormat interface for avro. However, the implementation class has a setter method for Avro schema since this not defined in EventFormat interface. Hence, following code would be used to get hold of format and serialize.

AvroFormat avroFormat = (AvroFormat) EventFormatProvider
        .getInstance()
        .resolveFormat(AvroFormat.CONTENT_TYPE);
//Set avro schema
avroFormat.setSchema(schema)

Let me know your thoughts.

@slinkydeveloper
Copy link
Member

avroFormat.setSchema(schema)

Which schema?

@subanasif
Copy link

This is avro schema. That is org.apache.avro.Schema.
This could be coming from filesystem or remote repository like schema registry.

@slinkydeveloper
Copy link
Member

slinkydeveloper commented Dec 11, 2020

avro schema of what? of Cloudevents? The one from the spec https://github.com/cloudevents/spec/blob/v1.0/spec.avsc?

@subanasif
Copy link

Thats correct. It *.avsc schema

@slinkydeveloper
Copy link
Member

Shouldn't this be loaded statically? Can we ship this schema as a resource in the package and then load it statically?

@subanasif
Copy link

The schema at https://github.com/cloudevents/spec/blob/v1.0/spec.avsc is just a spec, whereas real schema would have much more details (in data part, with specific field definitions).
Hence loading statically as resource won't work. More often, these spec compliant schemas are stored at third-party registries.

@slinkydeveloper
Copy link
Member

slinkydeveloper commented Dec 11, 2020

TBH I'm not an expert of avro, so I can't tell. What I can say is that it sounds weird to me to mutate the internal state of EventFormat in order to provide a custom schema... My expectation is that EventFormat doesn't parse the body, but it just collects it in a raw form (like in json, where it collects the data as json node) and then the user process this raw data into the pojo she/he wants to.
I think you should check out this PR where they're working on improving the schema cloudevents/spec#613

@yuce
Copy link

yuce commented Dec 11, 2020

@subanasif https://github.com/cloudevents/spec/blob/v1.0/spec.avsc is compiled into Java before shipping, so dynamically loading it is not necessary (although possible). That Avro spec is used by other CE clients, so I don't think it can be changed as long as compatibility with CE 1.0 is desired.

@pdebuitlear
Copy link

@subanasif https://github.com/cloudevents/spec/blob/v1.0/spec.avsc is compiled into Java before shipping, so dynamically loading it is not necessary (although possible). That Avro spec is used by other CE clients, so I don't think it can be changed as long as compatibility with CE 1.0 is desired.

I'm not sure that's what is being suggested.
@subanasif you mentioned you had an implementation, any chance you could share it for some more context?

@pdebuitlear
Copy link

any update on this issue?

@alsonlu
Copy link

alsonlu commented Jun 28, 2021

Is there any update on if/when this will be released?

@slinkydeveloper
Copy link
Member

This is just waiting for a contribution to come 😄

@sunng87 sunng87 linked a pull request Jul 16, 2021 that will close this issue
3 tasks
@sunng87
Copy link

sunng87 commented Jul 16, 2021

Just let you know I have been working on an avro serde impl these days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants