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 incorrect logs when a message failed to be decoded with the writer schema #200

Merged

Conversation

BewareMyPower
Copy link
Contributor

Motivation

See
https://github.com/apache/pulsar-client-python/blob/f9b2d168ae85f289d6ee043cd81791d569ba8844/pulsar/schema/schema_avro.py#L92C32-L92C69

When self._decode_bytes(msg.data(), writer_schema) failed, the error log is still Failed to get schema info, which is confusing.

Modifications

Modify the error message. Even if it failed at
self._get_writer_schema(topic, version), there would still be error logs from the C++ client.

@BewareMyPower BewareMyPower added the enhancement New feature or request label Feb 18, 2024
@BewareMyPower BewareMyPower added this to the 3.5.0 milestone Feb 18, 2024
@BewareMyPower BewareMyPower self-assigned this Feb 18, 2024
@BewareMyPower
Copy link
Contributor Author

It seems the main branch is broken. I will fix it first.

@BewareMyPower BewareMyPower marked this pull request as draft February 18, 2024 13:01
…r schema

### Motivation

See
https://github.com/apache/pulsar-client-python/blob/f9b2d168ae85f289d6ee043cd81791d569ba8844/pulsar/schema/schema_avro.py#L92C32-L92C69

When `self._decode_bytes(msg.data(), writer_schema)` failed, the error
log is still `Failed to get schema info`, which is confusing.

### Modifications

Modify the error message. Even if it failed at
`self._get_writer_schema(topic, version)`, there would still be error
logs from the C++ client.
@BewareMyPower BewareMyPower marked this pull request as ready for review February 19, 2024 02:15
@BewareMyPower BewareMyPower merged commit 48be179 into apache:main Feb 19, 2024
8 checks passed
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-error-msg branch February 19, 2024 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants