- Notifications
You must be signed in to change notification settings - Fork12
[typedb] forwarding state machine#115
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
whummer left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks great@purcell - much cleaner with the explicit state management viaForwardingState! 👍
| caseForwardingState.UNDECIDED: | ||
| self.buffer.append(data) | ||
| ifheaders:=get_headers_from_data_stream(self.buffer): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
small nitpick: I guess we could reverse the logic withif not ... andreturn, to reduce the level of indentation. (but really not critical, more a matter of taste / personal preference, so feel free to leave as-is :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, I think it could go either way. I personally find it clearer like this, but if the code grew longer, I'd switch it.
e20717d intomainUh oh!
There was an error while loading.Please reload this page.
This PR makes the state machine explicit in ForwardingBuffer, and also bumps the minimum Python version to 3.10, since the
matchstatement is used, and not available in 3.9.The minimum backend runtime version of Python for localstack seems to now be 3.13, so for maximum compatibility we should probably align that across all extensions?
(I also bumped the version of this extension as part of this PR.)