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

AVRO-3803: Python patched up ipc issues #2319

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

asosnovsky
Copy link

@asosnovsky asosnovsky commented Jul 5, 2023

AVRO-3803

What is the purpose of the change

  • Patched issue where FramedReader resulted in an infinite loop where it cannot read anything from the response (it was comparing bytes to a string)
  • Patched error handling for the Responder:
    • Invalid error source: except was pointing at schema.AvroException instead of avro.errors.AvroException
    • "Handshake" was lost for cases where the response could not be read due to things like "type errors"
  • Added some types to responder class

Verifying this change

This change added tests and can be verified as follows:

  • New test cases added
  • Type checker for Responder works (while we have tested it in our repo, I am honestly a bit confused on how to setup the local server with this code base)

Documentation

  • Does this pull request introduce a new feature? NO

@github-actions github-actions bot added the Python label Jul 5, 2023
lang/py/avro/schema.py Outdated Show resolved Hide resolved
lang/py/avro/schema.py Outdated Show resolved Hide resolved
@kojiromike
Copy link
Contributor

I think this is good, but can we attach it to any existing tickets? If not, I can open a new one.

@asosnovsky
Copy link
Author

@kojiromike no I don't have a JIRA for this. Please open one :) thank you!

@kojiromike kojiromike changed the title AVRO-0: Python patched up ipc issues AVRO-3803: Python patched up ipc issues Jul 19, 2023
@kojiromike
Copy link
Contributor

@asosnovsky could I trouble you to check if this implementation resolves AVRO-1639?

@kojiromike
Copy link
Contributor

As far as resolving the conflicts: If you take your branch as-is and run autoflake on it, that should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants