Skip to content

[schema] check if exists default constructor for avro/json schema#10395

Closed
linlinnn wants to merge 3 commits intoapache:masterfrom
linlinnn:schema-check
Closed

[schema] check if exists default constructor for avro/json schema#10395
linlinnn wants to merge 3 commits intoapache:masterfrom
linlinnn:schema-check

Conversation

@linlinnn
Copy link
Contributor

@linlinnn linlinnn commented Apr 27, 2021

Fixes #10393
Fixes #10377

Motivation
Check if exists default constructor for avro/json schema
Simplify code

Verify
Add unit test

@david-streamlio
Copy link
Contributor

LGTM

Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@congbobo184 congbobo184 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is a good change, avro api has checked this. we don't need to check this, give it to avro api is better.

@linlinnn
Copy link
Contributor Author

linlinnn commented Apr 28, 2021

I don't think it is a good change, avro api has checked this. we don't need to check this, give it to avro api is better.

@congbobo184
Could you point it out please?
I want to check this during init schema, otherwise will cause the issue that send message successfully but failed to consume message.

@Data
@AllArgsConstructor
public static class Payload {
    String message;
}

public static void main(String[] args) {
    AvroSchema<Payload> schema = AvroSchema.of(Payload.class);
    Payload payload = new Payload("test");
    ...
    producer.send(payload); // successfully
    Message<Payload> message = consumer.receive();
    message.getValue();      // exception
}

@congbobo184
Copy link
Contributor

congbobo184 commented Apr 28, 2021

@linlinnn Do we need to track every question about avro in advance? consume failed has given you an error about this, why we need to check in pulsar AvroSchema? You need to understand that this is not something pulsar needs to fix, it just avro problem.

@linlinnn linlinnn closed this Apr 28, 2021
@linlinnn
Copy link
Contributor Author

@congbobo184
From the side of design, I totally agree with your consideration.
My purpose is to help user being aware of this problem as far as possible, because it's common that there are no default constructor, even in pulsar test case.
I will try to fix this issue in arvo/json library side.

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.

Schema prevents usage of Dead Letter Queue DLQ Struct schema(AvroSchema/JSONSchema) cannot decode without default constructor.

5 participants