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

Use pb instead of protobin for binary descriptors #51

Merged
merged 1 commit into from
Jan 28, 2019

Conversation

clehene
Copy link

@clehene clehene commented Jan 18, 2019

Description
gcloud endpoints currently expects ['.pb', '.descriptor', '.proto.bin'] extensions or otherwise will fail with

UnicodeDecodeError: 'utf8' codec can't decode byte 0xae in position 1: invalid start byte
ERROR: gcloud crashed (UnicodeDecodeError): 'utf8' codec can't decode byte 0xae in position 1: invalid start byte

I've spent hours trying to figure how the file gets 'corrupted'..

I've also made a PR to look at the mime-type but it would probably be best to go with the "more default" extensions for descriptors.
https://github.com/google-cloud-sdk/google-cloud-sdk/pull/4

@sergei-ivanov sergei-ivanov self-requested a review January 28, 2019 00:34
@sergei-ivanov sergei-ivanov added this to the 0.7 milestone Jan 28, 2019
@sergei-ivanov sergei-ivanov self-assigned this Jan 28, 2019
@sergei-ivanov
Copy link
Member

protobin was a rather arbitrary choice back in the day, mainly because it did not clash with any other file extensions. At the time when we implemented support for generating binary protobuf descriptors, we could not find any guidelines on the file naming.
This PR contains changes that can potentially break backwards compatibility, but the next release is going to contain a number of breaking changes anyway.
While we are at it, I'd like to split off descriptor set generation into a goal of its own. Any objections to that?

@sergei-ivanov sergei-ivanov merged commit 18461bf into xolstice:master Jan 28, 2019
@sergei-ivanov
Copy link
Member

@clehene your PR to google-cloud-sdk/google-cloud-sdk#4 is probably not going to go far, because, as I understand, it is not an official repo for gcloud SDK.
I suggest that you raise an issue in Google's issue tracker instead:
https://issuetracker.google.com/issues?q=protobuf
You can reference your PR from there.

sergei-ivanov added a commit that referenced this pull request Jan 28, 2019
@clehene
Copy link
Author

clehene commented Jan 29, 2019

@sergei-ivanov thank you!

While we are at it, I'd like to split off descriptor set generation into a goal of its own. Any objections to that?

I think that may be useful

@clehene your PR to google-cloud-sdk/google-cloud-sdk#4 is probably not going to go far, because, as I understand, it is not an official repo for gcloud SDK.

Yes, I did notice it's just an automated mirror, but hoped they'd check the issues / PRs there too. I'll ask around and see what's the best place to push that.

@sergei-ivanov
Copy link
Member

#54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants