search

googleapis / python-pubsub

Apache License 2.0
390 stars 201 forks source link

Version 1.7.2 broken for Python 2.7 users #742

Closed wescpy closed 2 years ago

wescpy commented 2 years ago

Environment details

Also see:

DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
Package                  Version
------------------------ ---------
cachetools               3.1.1
certifi                  2021.10.8
chardet                  4.0.0
click                    7.1.2
enum34                   1.1.10
Flask                    1.1.4
futures                  3.3.0
google-api-core          1.32.0
google-auth              1.35.0
google-cloud-core        1.7.3
google-cloud-datastore   1.15.5
google-cloud-ndb         1.11.1
google-cloud-pubsub      1.7.2
googleapis-common-protos 1.52.0
grpc-google-iam-v1       0.12.3
grpcio                   1.39.0
idna                     2.10
itsdangerous             1.1.0
Jinja2                   2.11.3
MarkupSafe               1.1.1
packaging                20.9
protobuf                 3.17.3
pyasn1                   0.4.8
pyasn1-modules           0.2.8
pymemcache               3.5.2
pyparsing                2.4.7
pytz                     2022.1
redis                    3.5.3
requests                 2.27.1
rsa                      4.5
setuptools               44.1.1
six                      1.16.0
urllib3                  1.26.10
Werkzeug                 1.0.1

Steps to reproduce

  1. Grab the code in this folder
  2. Extract the code from main.py and convert this to a cmd-line app that can be easily verified, else deploy this to Python 2 App Engine.
  3. The offending code is in the log() function, which fails when trying to close the subscriber client (psc_client). Remove my AttributeError exception handler before you run it, and you should get one of the exceptions below when you execute it.

The issue is this line of v1.7.2 of the Pub/Sub client library. Basically self.api.transport.channel doesn't have a close attribute, so this call fails. It doesn't work as a context manager nor a plain call to close():

Using context manager (with statement):

Exception on /log [GET]
Traceback (most recent call last):
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/flask/app.py", line 1821, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/main.py", line 74, in log_visitors
    psc_client.acknowledge(subscription=SUB_PATH, ack_ids=acks)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/google/cloud/pubsub_v1/subscriber/client.py", line 259, in __exit__
    self.close()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t190652.445211499724902709/lib/google/cloud/pubsub_v1/subscriber/client.py", line 253, in close
    self.api.transport.channel.close()
AttributeError: 'Channel' object has no attribute 'close'

Using normal close():

Exception on /log [GET]
Traceback (most recent call last):
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/flask/app.py", line 1821, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/main.py", line 76, in log_visitors
    psc_client.close()
  File "/base/data/home/apps/s~PROJECT_ID/20220722t191304.445211601579102162/lib/google/cloud/pubsub_v1/subscriber/client.py", line 253, in close
    self.api.transport.channel.close()
AttributeError: 'Channel' object has no attribute 'close'

Code example

(see main.py in above code sample)

Stack trace

(see above)

Root cause speculation

Call to self.api.transport.channel.close() fails with a subscriber (subscription client). Either you should fix self.api.transport.channel so this call doesn't fail or wrap this close() call with an exception handler at that lower-level, basically backporting possible fixes from future versions of this module. Users have no idea what to do except to add extra code to suppress AttributeError here, and this is not a documented use case for this library—by that I mean this is not a user error, and they shouldn't be expected to write an exception handler here.

The problem is that the proper comms channels should actually be closed in this call. I also thought that channel has been deprecated and that transport s/b used, so I don't know why the call is like this.

Fix recommendation

To summarize, the offending code is this line: https://github.com/googleapis/python-pubsub/blob/v1.7.2/google/cloud/pubsub_v1/subscriber/client.py#L253

CURRENTLY:

def close(self):
        """Close the underlying channel to release socket resources.
        After a channel has been closed, the client instance cannot be used
        anymore.
        This method is idempotent.
        """
        self.api.transport.channel.close()

So my code doesn't throw this exception, I had to make this...

TEMPORARY BUT BAD FIX:

def close(self):
        """Close the underlying channel to release socket resources.
        After a channel has been closed, the client instance cannot be used
        anymore.
        This method is idempotent.
        """
        try:
            self.api.transport.channel.close()
        except AttributeError:
            pass

The real fix is to get the correct object in line 253 that needs to be closed and really close and release the gRPC communication resource, whether channel, transport, or something else. Again, you may need to investigate a future version of this file and backport the minimal necessary to get this to work for all remaining Python 2 Pub/Sub users out there. So far, it seems to only affect (synchronous pull) subscriber clients.

acocuzzo commented 2 years ago

Hi @wescpy I was able to run the following code without exception:

def main():
    # tally recent visitor counts from queue then delete those tasks
    tallies = {}
    acks = set()
    #with psc_client:
    print("pull")
    rsp = psc_client.pull(subscription=SUB_PATH, max_messages=1)
    msgs = rsp.received_messages
    for rcvd_msg in msgs:
        acks.add(rcvd_msg.ack_id)
        visitor = rcvd_msg.message.data.decode('utf-8')
        tallies[visitor] = tallies.get(visitor, 0) + 1
    if acks:
        psc_client.acknowledge(subscription=SUB_PATH, ack_ids=acks)
    psc_client.close()

    # increment those counts in Datastore and return
    if tallies:
        with ds_client.context():
            for visitor in tallies:
                counter = VisitorCount.query(VisitorCount.visitor == visitor).get()
                if not counter:
                    counter = VisitorCount(visitor=visitor, counter=0)
                    counter.put()
                counter.counter += tallies[visitor]
                counter.put()
    return 'DONE (with %d task[s] logging %d visitor[s])\r\n' % (len(msgs), len(tallies))

here is my pip freeze:

cachetools==3.1.1
certifi==2021.10.8
chardet==4.0.0
click==7.1.2
enum34==1.1.10
Flask==1.1.4
futures==3.3.0
google-api-core==1.32.0
google-auth==1.35.0
google-cloud-core==1.7.3
google-cloud-datastore==1.15.5
google-cloud-ndb==1.11.1
google-cloud-pubsub==1.7.2
googleapis-common-protos==1.52.0
grpc-google-iam-v1==0.12.3
grpcio==1.39.0
idna==2.10
itsdangerous==1.1.0
Jinja2==2.11.3
MarkupSafe==1.1.1
packaging==20.9
protobuf==3.17.3
pyasn1==0.4.8
pyasn1-modules==0.2.8
pymemcache==3.5.2
pyparsing==2.4.7
pytz==2022.1
redis==3.5.3
requests==2.27.1
rsa==4.5
six==1.16.0
urllib3==1.26.10
Werkzeug==1.0.1
wescpy commented 2 years ago

Thanks for the follow-up. Let me take a look at what you did and see if I can replicate it in my dev env then try to get it to run on the GAE platform where I encountered this issue. There's a good possibility that the grpcio I'm using (v1.39) conflicts with the one available on App Engine servers (v1.0.0).

acocuzzo commented 2 years ago

FYI I was using virtualenv to run, which I highly recommend if you can.

wescpy commented 2 years ago

Thx for the reco... using virtualenv was useful. I too, was able to get a cmd-line version working (w/grpcio 1.39) on my local Python 2 dev environment. Furthermore, when I rollback to grpcio==1.0.0, I was able to repro the error. Using successive "binary tree" testing, I discovered the fix for this bug was integrated in grpcio version 1.12.0 with this PR fixing this bug.

Conclusions:

  1. This issue I raised only exists for Pub/Sub Python 2 client library 1.7.2 users who have grpcio<1.12.0, and so long as grpcio>=1.12.0 is a prerequisite to install this version of the client library (or a proper warning given in its README), then you can close this issue as "WaI" as it's more of a grpcio issue vs. python-pubsub mainly a library-compatibility thing.
  2. This issue does still exist for all Python 2 App Engine users however, as the build system has hardcoded grpcio==1.0.0. I will file a bug for the App Engine team to upgrade to >=1.12.0.
wescpy commented 2 years ago

UPDATE: I filed a bug against the App Engine team to upgrade the built-in version of grpcio, but in the meantime, all users who run into this issue with their Python 2 App Engine apps using Cloud Pub/Sub can just "push" the exception handler I outlined earlier up to their code, e.g., don't use the context manager for the subscriber client, and wrap the close():

    try:
        psc_client.close()
    except AttributeError:  # special Py2 handler for grpcio<1.12.0
        pass