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

Python tests are broken in branch-2.10 #20314

Closed
lhotari opened this issue May 12, 2023 · 5 comments
Closed

Python tests are broken in branch-2.10 #20314

lhotari opened this issue May 12, 2023 · 5 comments
Labels
area/ci release/blocker Indicate the PR or issue that should block the release until it gets resolved release/2.10.5 Stale type/bug The PR fixed a bug or issue reported a bug

Comments

@lhotari
Copy link
Member

lhotari commented May 12, 2023

The python tests seem to break because of Python 2.7 compatibility issues

======================================================================
ERROR: test_python_instance (unittest.loader.ModuleImportFailure)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_python_instance
Traceback (most recent call last):
  File "/usr/lib/python2.7/unittest/loader.py", line 254, in _find_tests
    module = self._get_module_from_name(name)
  File "/usr/lib/python2.7/unittest/loader.py", line 232, in _get_module_from_name
    __import__(name)
  File "/pulsar/pulsar-functions/instance/target/python-instance/tests/test_python_instance.py", line 28, in <module>
    from python_instance import InstanceConfig
  File "/pulsar/pulsar-functions/instance/target/python-instance/python_instance.py", line 44, in <module>
    import state_context
  File "/pulsar/pulsar-functions/instance/target/python-instance/state_context.py", line 26, in <module>
    from bookkeeper import admin, kv
  File "/usr/local/lib/python2.7/dist-packages/bookkeeper/admin/__init__.py", line 15, in <module>
    from bookkeeper.admin.client import Client
  File "/usr/local/lib/python2.7/dist-packages/bookkeeper/admin/client.py", line 20, in <module>
    from bookkeeper import types
  File "/usr/local/lib/python2.7/dist-packages/bookkeeper/types.py", line 27, in <module>
    from bookkeeper.common.protobuf_helpers import get_messages
  File "/usr/local/lib/python2.7/dist-packages/bookkeeper/common/protobuf_helpers.py", line 18, in <module>
    from collections.abc import Mapping
ImportError: No module named abc

it seems to be related to apache/bookkeeper#3875 changes.

@zymap do you have a solution for fixing the Python tests in branch-2.10?

@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug area/ci labels May 12, 2023
@lhotari lhotari added release/blocker Indicate the PR or issue that should block the release until it gets resolved release/2.10.5 labels May 12, 2023
@hangc0276
Copy link
Contributor

This change was only released in BookKeeper 4.16.0 apache/bookkeeper#3875. Pulsar branch-2.10 uses BookKeeper 4.14.7, which doesn't contain this PR.

@mattisonchao
Copy link
Member

mattisonchao commented May 14, 2023

I wonder if we should maintain the Python client in the previous branches.
https://github.com/apache/pulsar-client-python
/cc @BewareMyPower @shibd

@zymap
Copy link
Member

zymap commented May 15, 2023

@lhotari We are fetching the latest bookkeeper in the python tests.

Collecting apache-bookkeeper-client>=4.9.2; extra == "all"
  Downloading apache_bookkeeper_client-4.16.1-py2.py3-none-any.whl (62 kB)

We replaced the python collections to collections.abc in apache/bookkeeper#3875 and released in 4.16. So it breaks the python test.

In Pulsar, we need to use a fixed bookkeeper version to avoid getting the latest version.

zymap added a commit to zymap/pulsar that referenced this issue May 15, 2023
---

Fixes: apache#20314

### Motivation

Use the fixed version of bookie-client to avoid some
break changes.
@BewareMyPower
Copy link
Contributor

BewareMyPower commented May 15, 2023

@mattisonchao See my previous discussion here.

@zymap @lhotari I think we can change the way to install the Python client for functions. Currently, it's installed by:

pip install pulsar-client[functions]

However, it's equivalent to:

pip install pulsar-client
pip install <some-other-dependencies>...

Because the dependencies by the extra [functions] package are never related to the Python client itself. Once we have available dependencies, we can install them manually:

pip install pulsar-client
# I assumes BK client 4.16.0 is still compatible with Python 2.7
pip install apache-bookkeeper-client==4.16.0
# TODO: pip install other dependencies

@github-actions
Copy link

The issue had no activity for 30 days, mark with Stale label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci release/blocker Indicate the PR or issue that should block the release until it gets resolved release/2.10.5 Stale type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

6 participants