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 potential NoneType access in aiokafka driver #569

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

GodTamIt
Copy link
Contributor

Description

This logic is likely meant to be a boolean OR not AND, since the first check in the statement checks for assignment to be None.

@wbarnha wbarnha self-requested a review November 21, 2023 00:55
Copy link
Member

@wbarnha wbarnha left a comment

Choose a reason for hiding this comment

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

Genuinely surprised how such a simple thing got overlooked for so long. Makes sense to change it to this.

What caused you to stumble upon this, out of curiosity @GodTamIt?

@wbarnha
Copy link
Member

wbarnha commented Nov 21, 2023

Ah crud... looks like I need to go clean up the pyproject.toml file. I promised myself I'd finish switching things over eventually, but now my hand is forced.

@GodTamIt
Copy link
Contributor Author

Genuinely surprised how such a simple thing got overlooked for so long. Makes sense to change it to this.

What caused you to stumble upon this, out of curiosity @GodTamIt?

@wbarnha I was running the test in #568 for long enough that it hit this edge case.

It was quite hard to reproduce actually.

@wbarnha
Copy link
Member

wbarnha commented Nov 21, 2023

Genuinely surprised how such a simple thing got overlooked for so long. Makes sense to change it to this.

What caused you to stumble upon this, out of curiosity @GodTamIt?

@wbarnha I was running the test in #568 for long enough that it hit this edge case.

It was quite hard to reproduce actually.

Thank you for finding this! I'll merge it ASAP.

@wbarnha wbarnha merged commit 448a7a4 into faust-streaming:master Nov 21, 2023
8 of 20 checks passed
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.

2 participants