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

Fix git req sanitization due to underscore #15607

Closed
wants to merge 1 commit into from

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Oct 29, 2024

SUMMARY

This is what @TheRealHaoLiu was asking about.

I'd suggest a followup change to the awx-plugins.interfaces library to also use -. Ping @webknjaz

https://github.com/ansible/awx_plugins.interfaces

I believe this is the right thing to do, because:

python-poetry/poetry#8848

that is the correctly normalized package name, not a bug

referencing its own source

https://peps.python.org/pep-0503/#normalized-names

This PEP references the concept of a “normalized” project name. As per PEP 426 the only valid characters in a name are the ASCII alphabet, ASCII numbers, ., -, and _. The name should be lowercased with all runs of the characters ., -, or _ replaced with a single - character. This can be implemented in Python with the re module:

And it writes code, in case that wasn't clear enough what normalization means.

import re

def normalize(name):
    return re.sub(r"[-_.]+", "-", name).lower()
ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API

@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Oct 29, 2024
Copy link

sonarcloud bot commented Oct 29, 2024

@webknjaz
Copy link
Member

@AlanCoding FTR, additionally wheel and sdist filenames are getting normalized slightly differently. Plus, that changed slightly earlier this year, so some downstreams had fun with https://warehouse.pypa.io/api-reference/integration-guide.html#predictable-urls. The way PyPI normalizes the project names is closer to sdists IIRC. I think this is what it uses: https://github.com/pypa/packaging/blob/029f415/src/packaging/utils.py#L46-L51.

I'd suggest a followup change to the awx-plugins.interfaces library to also use -.

We wouldn't want that. The reason it's underscored was that I wanted consistency with the importable namespace. So people would be less confused seeing that a PyPI project looks like awx_plugins.type.thing and what they reference in code is also awx_plugins.type.thing.
We are about to have 49 plugins namespaced as awx_plugins.credential.x / awx_plugins.managed_credential_type.y / awx_plugins.inventory.z and I wanted consistency in how what we control is shaped.

Though, if you still feel like this should be different, let's talk about it sooner rather than later. The plugin split is close and once I unblock it, it's going to be infinitely harder to change this.

@AlanCoding
Copy link
Member Author

Then that would suggest this be solved by a special case in the updater script. That's fine, I just wanted to put up these specifics.

@AlanCoding AlanCoding closed this Oct 29, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants