-
Notifications
You must be signed in to change notification settings - Fork 933
Support encoding with fastavro #492
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
Conversation
cf2b83b to
5d00428
Compare
| # magic byte | ||
| outf.write(struct.pack('b', MAGIC_BYTE)) | ||
|
|
||
| # write the schema ID in network byte order (big end) |
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.
nit: this comment still applies though
edenhill
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.
LGTM
| # get the writer | ||
| writer = self.id_to_writers[schema_id] | ||
| with ContextStringIO() as outf: | ||
| # write the header |
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.
nit: these three comment lines read strange now. combine them into one.
| # write the schema ID in network byte order (big end) | ||
|
|
||
| outf.write(struct.pack('>I', schema_id)) | ||
| # Write the magic byte and schema ID in network byte order (big end) |
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.
micronit: "big endian" (from original comment though)
| writer = self.id_to_writers[schema_id] | ||
| with ContextStringIO() as outf: | ||
| # Write the magic byte and schema ID in network byte order (big end) | ||
| # Write the magic byte and schema ID in network byte order (big endian) |
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.
Great fix!
wrap up work contributed by #419