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

chore: Remove restriction that protobuf should be less than 5 #1193

Merged
merged 3 commits into from
Jun 20, 2024

Conversation

mukund-ananthu
Copy link
Contributor

@mukund-ananthu mukund-ananthu commented Jun 14, 2024

I see that an upper bound of 5 was set on the protobuf library in 23ce35e

I do not find any documentation on reasons for keeping an upper bound. Proposing removing the upper bound in this PR, but also checking if there are any reasons we would not want to do this from @parthea who would have historical context on this.

Fixes #1192 🦕

BEGIN_COMMIT_OVERRIDE
fix: allow Protobuf 5.x
END_COMMIT_OVERRIDE

@mukund-ananthu mukund-ananthu requested review from a team as code owners June 14, 2024 00:50
@mukund-ananthu mukund-ananthu self-assigned this Jun 14, 2024
@mukund-ananthu
Copy link
Contributor Author

@parthea PTAL when free.

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/python-pubsub API. label Jun 14, 2024
@jeffsawatzky
Copy link

I would very much like to have this fixed as well, for similar reasons to #1192
@mukund-ananthu / @parthea this is currently blocking me due to dependency hell. If you could remove this restriction (or loosen it to <6.0.0dev for now) it would make things much easier. Thanks!

setup.py Outdated Show resolved Hide resolved
Co-authored-by: Anthonios Partheniou <partheniou@google.com>
@product-auto-label product-auto-label bot added the size: xs Pull request size is extra small. label Jun 19, 2024
@parthea
Copy link
Contributor

parthea commented Jun 19, 2024

Also update testing/constraints-3.7.txt

Change

to

protobuf==3.20.2

@parthea
Copy link
Contributor

parthea commented Jun 19, 2024

The system tests are failing due to an unrelated issue.
See https://github.com/googleapis/python-pubsub/issues/created_by/app/flaky-bot

___________ test_subscriber_not_leaking_open_sockets[rest-rest-rest] ___________

publisher = 
topic_path_base = 'projects/<REDACTED>/topics/<REDACTED>'
subscription_path_base = 'projects/<REDACTED>/subscriptions/<REDACTED>'
cleanup = [(>, (), {'topic': 'projects/<REDACTED>/topics/<REDACTED>'})]
transport = 'rest'

    @pytest.mark.parametrize("transport", ["grpc", "rest"])
    def test_subscriber_not_leaking_open_sockets(
        publisher, topic_path_base, subscription_path_base, cleanup, transport
    ):
        # Make sure the topic and the supscription get deleted.
        # NOTE: Since subscriber client will be closed in the test, we should not
        # use the shared `subscriber` fixture, but instead construct a new client
        # in this test.
        # Also, since the client will get closed, we need another subscriber client
        # to clean up the subscription. We also need to make sure that auxiliary
        # subscriber releases the sockets, too.
        custom_str = "-not-leaking-open-sockets"
        subscription_path = subscription_path_base + custom_str
        topic_path = topic_path_base + custom_str
        subscriber = pubsub_v1.SubscriberClient(transport="grpc")
        subscriber_2 = pubsub_v1.SubscriberClient(transport="grpc")
    
        cleanup.append(
            (subscriber_2.delete_subscription, (), {"subscription": subscription_path})
        )
        cleanup.append((subscriber_2.close, (), {}))
        cleanup.append((publisher.delete_topic, (), {"topic": topic_path}))
    
        # Create topic before starting to track connection count (any sockets opened
        # by the publisher client are not counted by this test).
        publisher.create_topic(name=topic_path)
    
        current_process = psutil.Process()
>       conn_count_start = len(current_process.connections())

tests/system.py:471: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = psutil.Process(pid=397, name='py.test', status='running', started='16:34:59')
args = (), kwargs = {}

    @functools.wraps(fun)
    def inner(self, *args, **kwargs):
>       warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
E       DeprecationWarning: connections() is deprecated and will be removed; use net_connections() instead

@mukund-ananthu mukund-ananthu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 19, 2024
@mukund-ananthu mukund-ananthu force-pushed the protobufVersion branch 2 times, most recently from 3df084a to 86ecee0 Compare June 19, 2024 23:47
@mukund-ananthu mukund-ananthu merged commit a369f04 into main Jun 20, 2024
28 checks passed
@mukund-ananthu mukund-ananthu deleted the protobufVersion branch June 20, 2024 15:14
@parthea parthea added the release-please:force-run To run release-please label Jun 20, 2024
@release-please release-please bot removed the release-please:force-run To run release-please label Jun 20, 2024
@parthea parthea added the release-please:force-run To run release-please label Jun 20, 2024
@release-please release-please bot removed the release-please:force-run To run release-please label Jun 20, 2024
@mukund-ananthu
Copy link
Contributor Author

mukund-ananthu commented Jun 20, 2024

@jeffsawatzky and @cglewis . The fix is submitted and available in the latest release 2.21.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/python-pubsub API. size: xs Pull request size is extra small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use patched version of grpcio with google-cloud-pubsub
4 participants