Skip to content

Kafka codec: refactor (nullable)string/bytes deserializers so they ut…#6851

Merged
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
adamkotwasinski:kafka-serializer-improvements
May 8, 2019
Merged

Kafka codec: refactor (nullable)string/bytes deserializers so they ut…#6851
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
adamkotwasinski:kafka-serializer-improvements

Conversation

@adamkotwasinski
Copy link
Contributor

…ilize the same feed code

There were 4 deserializers that were basically doing the same thing:

  • read payload length
  • initialize buffer with size == length and handle null value
  • put more bytes into buffer

This commit refactors all of that so the code stays in one place.

Discussed before in:
#6831 (review)
and
#4950 (comment)

Description:
Risk Level: Low (new Kafka codec that's not active anyways)
Testing: automated tests
Docs Changes: n/a
Release Notes: n/a

…ilize the same feed code

Signed-off-by: Adam Kotwasinski <adam.kotwasinski@gmail.com>
@mattklein123 mattklein123 self-assigned this May 7, 2019
@adamkotwasinski
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: release (failed build)

🐱

Caused by: a #6851 (comment) was created by @adamkotwasinski.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the cleanup. Tiny nit.

/wait

* @return number of bytes consumed.
*/
template <typename DeserializerType, typename LengthType, typename ByteType>
size_t feed_bytes_into_buffers(absl::string_view& data, DeserializerType& length_deserializer,
Copy link
Member

Choose a reason for hiding this comment

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

nit: feedBytesIntoBuffers

Signed-off-by: Adam Kotwasinski <adam.kotwasinski@gmail.com>
@adamkotwasinski
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: release (failed build)

🐱

Caused by: a #6851 (comment) was created by @adamkotwasinski.

see: more, trace.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thank you!

@mattklein123 mattklein123 merged commit f0b508f into envoyproxy:master May 8, 2019
@adamkotwasinski adamkotwasinski deleted the kafka-serializer-improvements branch May 8, 2019 20:08
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