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

Raise KeyError in Python ServiceDescriptor.FindMethodByName (#9592) #9998

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

tomerv
Copy link
Contributor

@tomerv tomerv commented May 19, 2022

  • Align Python version with C-extension version
    (python/google/protobuf/pyext/descriptor.cc)
  • Update documentation accordingly
  • Add unit test

@tomerv
Copy link
Contributor Author

tomerv commented May 29, 2022

@mkruskal-google please trigger another kokoro run, I fixed the test that failed before.
@anandolee please review
Thanks!

@tomerv
Copy link
Contributor Author

tomerv commented Jun 7, 2022

How can I move this forward?
Seems like there's some problem with "MacOS Python Release" build that isn't related to my changes (but maybe it is? How do I find out?). And also I need someone to look at the actual changes.
Thanks.

@mkruskal-google
Copy link
Member

How can I move this forward? Seems like there's some problem with "MacOS Python Release" build that isn't related to my changes (but maybe it is? How do I find out?). And also I need someone to look at the actual changes. Thanks.

I suspect this is a false alarm. We recently deleted the python release builds, but github doesn't automatically clear deleted tests and just keeps displaying the old result. Please rebase your branch to refresh these, and I can rerun the tests. I'll also add a reviewer, as I'm not familiar enough with python proto to determine if this is safe or not

@mkruskal-google mkruskal-google requested a review from haberman June 7, 2022 15:44
…buffers#9592)

* Align Python version with C-extension version
  (python/google/protobuf/pyext/descriptor.cc)
* Update documentation accordingly
* Add unit test
@tomerv
Copy link
Contributor Author

tomerv commented Jun 7, 2022

Please rebase your branch to refresh these

Done.

@tomerv
Copy link
Contributor Author

tomerv commented Jun 15, 2022

@haberman , @mkruskal-google , sorry to be so pushy, but...

https://github.com/protocolbuffers/protobuf/blob/main/CONTRIBUTING.md?plain=1#L66

We will try to keep our response time within 7-days but if you don’t get any response in a few days, feel free to comment on the threads to get our attention.

🙂

@esorot esorot merged commit f9ebb9e into protocolbuffers:main Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants