8000 Fix distribution name normalization (PEP-0503) by rominf · Pull Request #2144 · getsentry/sentry-python · GitHub
[go: up one dir, main page]
Skip to content

Fix distribution name normalization (PEP-0503) #2144

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.

Sign up for GitHub

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

Jump to bottom
Merged
sentrivana merged 1 commit into from
May 25, 2023
Merged

Fix distribution name normalization (PEP-0503) #2144

sentrivana merged 1 commit into from
May 25, 2023

Conversation

@rominf
Copy link
Contributor
@rominf rominf commented May 25, 2023

Current logic in test_installed_modules does not properly handle
distributions with underscores. On my machine I get the following error
while running tests:

tests/integrations/modules/test_modules.py:60: in test_installed_modules
    assert installed_modules == pkg_resources_modules
E   AssertionError: assert {'aiven-clien...'22.2.0', ...} == {'aiven-clien...'22.2.0', ...}
E     Omitting 93 identical items, use -vv to show
E     Left contains 1 more item:
E     {'tomli_w': '1.0.0'}
E     Right contains 1 more item:
E     {'tomli-w': '1.0.0'}
E     Use -v to get more diff

This change fixes distribution name normalization by applying the code
from PEP-0503 (https://peps.python.org/pep-0503/#normalized-names).

@rominf
Copy link
Contributor Author
rominf commented May 25, 2023 •
edited
Loading

I get the same error while building Fedora package on Fedora servers: https://kojipkgs.fedoraproject.org//work/tasks/659/101530659/build.log

=================================== FAILURES ===================================
____________________________ test_installed_modules ____________________________
tests/integrations/modules/test_modules.py:60: in test_installed_modules
    assert installed_modules == pkg_resources_modules
E   AssertionError: assert {'aiohttp': '... '3.5.0', ...} == {'aiohttp': '... '3.5.0', ...}
E     Omitting 113 identical items, use -vv to show
E     Left contains 1 more item:
E     {'tomli_w': '1.0.0'}
E     Right contains 1 more item:
E     {'tomli-w': '1.0.0'}
E     Use -v to get more diff

This prevents me from packaging newest version of python3-sentry-sdk for Fedora.

@rominf rominf force-pushed the fix-test_installed_modules branch 2 times, most recently from bf039e9 to a6fc449 Compare May 25, 2023 13:41
@rominf rominf marked this pull request as ready for review May 25, 2023 13:42
Fix distribution name normalization (PEP-0503)
3d6b8f6 
Current logic in `test_installed_modules` does not properly handle
distributions with underscores. On my machine I get the following error
while running tests:
```
tests/integrations/modules/test_modules.py:60: in test_installed_modules
    assert installed_modules == pkg_resources_modules
E   AssertionError: assert {'aiven-clien...'22.2.0', ...} == {'aiven-clien...'22.2.0', ...}
E     Omitting 93 identical items, use -vv to show
E     Left contains 1 more item:
E     {'tomli_w': '1.0.0'}
E     Right contains 1 more item:
E     {'tomli-w': '1.0.0'}
E     Use -v to get more diff
```

This change fixes distribution name normalization by applying the code
from PEP-0503 (https://peps.python.org/pep-0503/#normalized-names).
@rominf rominf force-pushed the fix-test_installed_modules branch from a6fc449 to 3d6b8f6 Compare May 25, 2023 13:51
@sentrivana
Copy link
Contributor
sentrivana commented May 25, 2023

Hey @rominf, thanks a lot for fixing this! If the tests are green this LGTM.

@sentrivana sentrivana enabled auto-merge (squash) May 25, 2023 13:55
@sentrivana sentrivana merged commit a48a3bb into getsentry:master May 25, 2023
@rominf rominf deleted the fix-test_installed_modules branch May 25, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sentrivana sentrivana sentrivana approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rominf @sentrivana
0