Skip to content

Conversation

hashworks
Copy link
Collaborator

@hashworks hashworks force-pushed the chore/debian-bookworm branch from 19a2bfa to 6297cec Compare February 1, 2025 13:37
@dimitri dimitri requested a review from df7cb February 3, 2025 12:12
@dimitri dimitri added good first issue Good for newcomers Developer productivity Enhancements to ability to ship quality code Packaging and CI Enhancements to our CI integration dependencies Pull requests that update a dependency file labels Feb 3, 2025
@df7cb
Copy link
Contributor

df7cb commented Feb 3, 2025

RUN pip3 install pyroute2>=0.5.17

This should be installed as a Debian package instead, it's version 0.7.2 in bookworm.

@hashworks
Copy link
Collaborator Author

hashworks commented Feb 3, 2025

RUN pip3 install pyroute2>=0.5.17

This should be installed as a Debian package instead, it's version 0.7.2 in bookworm.

Added in 0c61f11.

But I see there is a Pipfile... I had to look up what that is. As far as I see the project is dead, should I:
A) Simply remove Pipfile and Pipfile.lock since we expect people to use docker or
B) Convert it to requirements.txt ?

@hashworks hashworks force-pushed the chore/debian-bookworm branch from 0c61f11 to 6967272 Compare February 3, 2025 14:33
@dimitri
Copy link
Contributor

dimitri commented Feb 3, 2025

A Quick Look shows that the PIP file was used to install python packages pyroute2, nose, and psycopg2 all from pypi. I see that we have apt-get coverage for those 3 now in the Dockerfile (with your addition of pyroute2 there), so please see about removing the pipfiles entirely, and maybe also python3-pip from the package list installed with apt-get.

@dimitri dimitri enabled auto-merge (squash) February 3, 2025 16:47
@dimitri dimitri disabled auto-merge February 3, 2025 17:19
@hashworks
Copy link
Collaborator Author

Test failures seem to be due to the updated pyroute2 package, I'll take a look tomorrow.

ERROR: test_multi_ifdown.test_012_prepare_replication_quorums
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/src/pg_auto_failover/tests/test_multi_ifdown.py", line 207, in test_012_prepare_replication_quorums
    assert node2.set_replication_quorum("false")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/pg_auto_failover/tests/pgautofailover_utils.py", line 1516, in set_replication_quorum
    raise e
  File "/usr/src/pg_auto_failover/tests/pgautofailover_utils.py", line 1506, in set_replication_quorum
    command.execute(
  File "/usr/src/pg_auto_failover/tests/pgautofailover_utils.py", line 2030, in execute
    out, err = self.pgnode.cluster.communicate(proc, timeout)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/pg_auto_failover/tests/pgautofailover_utils.py", line 249, in communicate
    return proc.communicate(timeout=1)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/pyroute2/nslink/nspopen.py", line 224, in proxy
    return _handle(self.channel_in.get())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/pyroute2/nslink/nspopen.py", line 25, in _handle
    raise result['data']
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x90 in position 289: invalid start byte

@dimitri
Copy link
Contributor

dimitri commented Feb 4, 2025

Test failures seem to be due to the updated pyroute2 package, I'll take a look tomorrow.

My advice would then be to remove that change from this PR, and make another PR that modernizes the installation of Python dependencies for our unit testing.

@hashworks hashworks force-pushed the chore/debian-bookworm branch from 7ddd939 to 6664973 Compare February 4, 2025 12:36
@hashworks
Copy link
Collaborator Author

My advice would then be to remove that change from this PR, and make another PR that modernizes the installation of Python dependencies for our unit testing.

Moved to #1070.

This avoids conflicts with packages both in the postgres and in the main
debian package sources.

I.e. installing libpq before libpq-dev from the postgres sources can
lead to broken links for `libpq.so.5`.
@hashworks
Copy link
Collaborator Author

hashworks commented Feb 9, 2025

ERROR: test_multi_ifdown.test_012_prepare_replication_quorums
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/src/pg_auto_failover/tests/test_multi_ifdown.py", line 207, in test_012_prepare_replication_quorums
    assert node2.set_replication_quorum("false")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
  File "/usr/lib/python3/dist-packages/pyroute2/nslink/nspopen.py", line 224, in proxy
    return _handle(self.channel_in.get())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/pyroute2/nslink/nspopen.py", line 25, in _handle
    raise result['data']
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x90 in position 289: invalid start byte

I wasn't able to find a fix for this, if anyone has any ideas I would appreciate it.

@dimitri
Copy link
Contributor

dimitri commented Feb 10, 2025

On a first look I am not seeing anything related in the pyroute2 changelog at https://docs.pyroute2.org/changelog.html. From https://packages.debian.org/search?keywords=pyroute2 it seems we are upgrading from version 0.5.14 to version 0.7.2. Or it might be a Python 3.x upgrade thing with unicode support?

Also might be the bytes that are reported back from the pg_autoctl command. Did you try manually running the set replication quorum command on a test cluster?

@dimitri
Copy link
Contributor

dimitri commented Apr 3, 2025

Still having the same issues when updating python3/pyroute2 (UnicodeDecodeError). Let's go ahead with the release and see about modernizing our CI later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Developer productivity Enhancements to ability to ship quality code good first issue Good for newcomers Packaging and CI Enhancements to our CI integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants