Skip to content

downgrade to protobuf 3 to satisfy temporary workaround #184

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

Closed
wants to merge 3 commits into from

Conversation

jeremy-cho
Copy link

@jeremy-cho jeremy-cho commented Nov 2, 2022

What was changed

This PR essentially reverts the protobuf 4 upgrade changes that were merged into main as part of #136. The intent of this PR is to create a workaround for projects that incur unresolvable package dependency issues caused by the upgrade and is NOT meant to be merged into main.

Relevant Slack thread in Temporal Slack community: https://temporalio.slack.com/archives/CTT84RS0P/p1667336466199119

Why?

temporalio==0.1b2 introduced some nice changes like the testing module but also upgrades the protobuf dependency to protobuf < 4.22, >= 4.21 which can cause issues in existing projects that utilize packages that don't support protobuf 4. As a workaround, this PR downgrades the protobuf package and attempts to create a wheel that includes the 0.1b2 changes excluding the protobuf upgrade.

Checklist

  1. This PR doesn't close any issues but the relevant issue to track this is [Feature Request] Review options for allowing protobuf 3.x to work with this library #181

  2. How was this tested:
    As long as CI passes I think we're good?

  3. Any docs updates needed?
    None needed I believe

@CLAassistant
Copy link

CLAassistant commented Nov 2, 2022

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@cretz
Copy link
Member

cretz commented Nov 2, 2022

Approving to run CI. This made more changes than just the proto downgrade btw.

@jeremy-cho
Copy link
Author

Yep, thanks for pointing that out @cretz I'm still in the process of grooming the changes will poke you when I feel like it's ready

@jeremy-cho jeremy-cho force-pushed the main branch 3 times, most recently from df13b06 to c10e548 Compare November 2, 2022 21:11
@jeremy-cho
Copy link
Author

Okay I think it's in a state now where at least the files touched are the same as the update PR. Lines changed still look a little different but 🤷‍♂️

@cretz
Copy link
Member

cretz commented Nov 28, 2022

See #215 where I have added proto 3 support back into the library

@cretz
Copy link
Member

cretz commented Dec 13, 2022

@jeremy-cho-skydio - with proto 3 and proto 4 now both supported in the latest release, can we close this?

@cretz
Copy link
Member

cretz commented Jan 3, 2023

Closing PR. Can reopen if needed.

@cretz cretz closed this Jan 3, 2023
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