Skip to content

Switch to PEP 517 packaging using hatchling #2930

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

Merged
merged 8 commits into from
Feb 25, 2025
Merged

Conversation

akx
Copy link
Contributor

@akx akx commented Sep 4, 2023

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests pass?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?
  • Was the change added to CHANGES file?

Description of change

Quoting the description from #2388, a year later...

This PR shifts redis-py from legacy setup.py packaging to PEP 517 compliant packaging using hatchling from PyPA.

Essentially, this does what was discussed in #1658 but with the increasingly standard hatchling tool instead of poetry's idiosyncracies.

As with #2388, python -m build does create a working wheel with all modules included.

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (2f88840) 91.84% compared to head (835daaa) 91.84%.
Report is 5 commits behind head on master.

Files Patch % Lines
redis/asyncio/connection.py 83.33% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2930      +/-   ##
==========================================
- Coverage   91.84%   91.84%   -0.01%     
==========================================
  Files         128      127       -1     
  Lines       33232    33270      +38     
==========================================
+ Hits        30523    30557      +34     
- Misses       2709     2713       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@akx akx marked this pull request as ready for review February 15, 2024 12:58
@akx
Copy link
Contributor Author

akx commented Jun 12, 2024

Rebased.

@akx akx force-pushed the pep517-2023 branch 3 times, most recently from 7c9926b to 4dbdd76 Compare August 27, 2024 09:27
@akx
Copy link
Contributor Author

akx commented Aug 27, 2024

Rebased. Cc @gerzse @vladvildanov.

@vladvildanov
Copy link
Collaborator

@akx Thanks for your contribution! I'll review this PR in near time

@akx
Copy link
Contributor Author

akx commented Oct 16, 2024

@vladvildanov Good to hear. Rebased once again.

@akx akx force-pushed the pep517-2023 branch 2 times, most recently from cc362bc to ac7e7e7 Compare October 16, 2024 17:36
@akx akx requested a review from vladvildanov October 16, 2024 17:36
@akx akx marked this pull request as draft October 16, 2024 17:37
@akx akx force-pushed the pep517-2023 branch 2 times, most recently from b0fe33b to 697c349 Compare October 16, 2024 17:40
@akx akx marked this pull request as ready for review October 16, 2024 17:40
@akx akx force-pushed the pep517-2023 branch 2 times, most recently from dd4c138 to 2b65eff Compare February 20, 2025 10:58
@akx
Copy link
Contributor Author

akx commented Feb 20, 2025

@vladvildanov Rebased. I'll fix the tests if there's any interest in this.

@woutdenolf
Copy link
Contributor

Could you add "closes #3463 "?

@akx
Copy link
Contributor Author

akx commented Feb 21, 2025

Btw: the tests failing in

FAILED tests/test_asyncio/test_commands.py::TestRedisCommands::test_client_setinfo[single] - AssertionError: assert '5.3.0b5' == '5.2.1'
    
    - 5.2.1
    + 5.3.0b5

is very curious – given the string "5.3.0b5" does not appear in the source here at all, it almost looks like the CI pipeline is installing an entirely different version of the package...

and indeed:

  Downloading redis-5.3.0b5-py3-none-any.whl (271 kB)

IOW, the CI pipeline seems to be confused about which libraries it's using...

@akx akx force-pushed the pep517-2023 branch 3 times, most recently from 026dbe1 to f3cbaf2 Compare February 21, 2025 08:32
@akx akx requested a review from petyaslavova February 25, 2025 09:14
Copy link
Collaborator

@petyaslavova petyaslavova left a comment

Choose a reason for hiding this comment

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

LGTM

@vladvildanov vladvildanov added the maintenance Maintenance (CI, Releases, etc) label Feb 25, 2025
@petyaslavova petyaslavova merged commit 483c021 into redis:master Feb 25, 2025
37 checks passed
@akx
Copy link
Contributor Author

akx commented Feb 25, 2025

Thanks for getting this finally merged!

1 year, 5 months 3 weeks – could easily be a personal record for PR perseverance for me.

@akx akx deleted the pep517-2023 branch February 25, 2025 11:17
@vladvildanov
Copy link
Collaborator

@akx Thanks for your efforts, I'm so sorry that It took so long from our side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Maintenance (CI, Releases, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependency files and add dependencies to setup.py
5 participants