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

Make an opt-out path for the BINARY format #10

Merged
merged 3 commits into from
Aug 4, 2016
Merged

Conversation

bhs
Copy link
Contributor

@bhs bhs commented Aug 4, 2016

Make it possible for BasicTracer users to avoid even an import of the BinaryPropagator, as the latter can throw protobuf-related exceptions at initialization time.

cc: @bg451

bhs added 2 commits August 3, 2016 21:06
Make it possible for BasicTracer users to avoid even an *import* of the
BinaryPropagator, as the latter can throw protobuf-related exceptions at
initialization time.
_proto_size_bytes = 4 # bytes


class BinaryPropagator(Propagator):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the point of this change is that a BasicTracer extender may choose not to even import this BinaryPropagator file and its .wire_pb2 dependency that breaks some builds that also involve other version of protobufs.

@bhs
Copy link
Contributor Author

bhs commented Aug 4, 2016

See #2

register_required_propagators().

The required formats are opt-in because of protobuf version conflicts
with the binary carrior.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/carrior/carrier/

@bg451
Copy link
Contributor

bg451 commented Aug 4, 2016

One nit, otherwise lgtm.

@bhs bhs merged commit 7da4b1c into master Aug 4, 2016
@bhs bhs deleted the bhs/disable_binary_format branch August 4, 2016 23:11
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.

3 participants