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

Use psycopg(3) for Python 3.13 compatibility #49

Merged
merged 3 commits into from
Sep 4, 2024
Merged

Conversation

martinhoyer
Copy link
Contributor

Haven't done any testing yet, but at least it builds on F41 Python 3.13.

Required by tmt[test-convert]

@martinhoyer
Copy link
Contributor Author

looks like libpq is not being installed as a dependency of python3-psycopg3 package on certain distros.

@martinhoyer
Copy link
Contributor Author

fwiw epel-8 build should fail. I don't know what's in plan for this project in that regard.

@martinhoyer
Copy link
Contributor Author

@lukaszachy Would you have a moment to check this, if you'll have rights to this repo?

@lukaszachy
Copy link
Collaborator

fwiw epel-8 build should fail. I don't know what's in plan for this project in that regard.

But it succeeded...

Copy link
Collaborator

@lukaszachy lukaszachy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this

@martinhoyer
Copy link
Contributor Author

fwiw epel-8 build should fail. I don't know what's in plan for this project in that regard.

But it succeeded...

hmm, not sure how. Didn't study all the conditionals in spec file.
Regardless, the el8 tests: "⚠ Test environment installation failed: Install packages".

@martinhoyer
Copy link
Contributor Author

@lukaszachy soo, I got lost in the specfile, so I've trimmed it a bit. Hoping epel8 will be fine now. wdyt? 0938e7b

@martinhoyer
Copy link
Contributor Author

Also, I've noticed the tests were green, but it didn't look like they really ran. Let's see what happens with these changes

@martinhoyer martinhoyer requested a review from lukaszachy August 27, 2024 14:32
@martinhoyer
Copy link
Contributor Author

I don't get it. Where does it tell it to require psycopg and not psycopg2 on epel8?

nothing provides python3.6dist(psycopg) needed by python3-nitrate-1.9.0-1.20240827144344806669.pr49.14.gdb1f011.el8.noarch

%py3_build %py3_install?

@lukaszachy
Copy link
Collaborator

I'm trying to understand what does %python_enable_dependency_generator. Which would be my guess...

@lukaszachy
Copy link
Collaborator

lukaszachy commented Aug 27, 2024

@martinhoyer The value comes from setup.py ( I tried to put 'foo' there and it appeared in require as python3.6dist(foo)`)

$ make srpm
$ mock -r centos-stream+epel-8-x86_64 tmp/SRPMS/python-nitrate-1.9.0-1.fc40.src.rpm

@lukaszachy
Copy link
Collaborator

looks like libpq is not being installed as a dependency of python3-psycopg3 package on certain distros.

Which distro was that?

@lukaszachy
Copy link
Collaborator

@martinhoyer I pushed commit to your PR, please check:

  • use psycopg3 just for python3.13
  • let setup.py be the source for requires (I've tried in mock epel8 and it works... we don't care about rhel-7 anymore, right?)
  • remove the libpq explicit deps - f41 seems to have it fixed

@martinhoyer
Copy link
Contributor Author

looks like libpq is not being installed as a dependency of python3-psycopg3 package on certain distros.

Which distro was that?

I've re-tested it now. It's not being pulled on F40, EPEL9, while it's being installed on el10 (psycopq2) and F39

@martinhoyer
Copy link
Contributor Author

martinhoyer commented Sep 3, 2024

Sorry, I completely forgot about this PR. Thanks! I wanted to do basically what you did :)

@martinhoyer I pushed commit to your PR, please check:

* use psycopg3 just for python3.13

Sure.. or <3.12 to have all Fedoras on psycopg3.

* let setup.py be the source for requires (I've tried in mock epel8 and it works... we don't care about rhel-7 anymore, right?)

I hope so :)
Interesting that it even works. I don't have experience with "old" python rpm macros. Looks good, cheers.

* remove the libpq explicit deps - f41 seems to have it fixed

I'm guessing this will break the other distros mentioned above (F40 and EPEL9).

martinhoyer and others added 2 commits September 3, 2024 15:56
Python macro can use setup.py as the Requires source, lets have it on
single place.
@martinhoyer
Copy link
Contributor Author

@lukaszachy @psss Looks green :)

My only remaining point is "use psycopg2 on python<3.12" vs "use psycopg3 on python>3.13". The former make more sense to me - having it same on all current Fedoras, not just 41+... but no strong opinion.

@lukaszachy
Copy link
Collaborator

My only remaining point is "use psycopg2 on python<3.12" vs "use psycopg3 on python>3.13". The former make more sense to me - having it same on all current Fedoras, not just 41+... but no strong opinion.

Personally I'd prefer to keep using psycopg2 where it is available and make psycopg3 to appear with python3.13+ becoming the base python. If only we would not need to fix libpq deps where python3-psycopg3 isn't packaged correctly :)

@martinhoyer
Copy link
Contributor Author

My only remaining point is "use psycopg2 on python<3.12" vs "use psycopg3 on python>3.13". The former make more sense to me - having it same on all current Fedoras, not just 41+... but no strong opinion.

Personally I'd prefer to keep using psycopg2 where it is available and make psycopg3 to appear with python3.13+ becoming the base python. If only we would not need to fix libpq deps where python3-psycopg3 isn't packaged correctly :)

libpq is not being installed as a dep even with psycopg2 on el9 from what can tell

Copy link
Owner

@psss psss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@lukaszachy
Copy link
Collaborator

libpq is not being installed as a dep even with psycopg2 on el9 from what can tell

Hm,.. I've just tested in VM and

Installing:
 python3-psycopg2                   x86_64                   2.8.6-6.el9                    rhel-AppStream                   192 k
Installing dependencies:
 libpq                              x86_64                   13.15-1.el9                    rhel-AppStream                   211 k

@psss psss merged commit e723380 into psss:main Sep 4, 2024
13 checks passed
@martinhoyer
Copy link
Contributor Author

libpq is not being installed as a dep even with psycopg2 on el9 from what can tell

Hm,.. I've just tested in VM and

Installing:
 python3-psycopg2                   x86_64                   2.8.6-6.el9                    rhel-AppStream                   192 k
Installing dependencies:
 libpq                              x86_64                   13.15-1.el9                    rhel-AppStream                   211 k

hmm yeah, thanks for that correction. I must have confused myself with all the psycopgs everywhere.

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