Skip to content

fix(pypi): update get_maintainers_of_package to avoid request blocking #1097

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AmineRaouane
Copy link
Member

Summary

This PR improves the functionality of the pypi_registry.py by enhancing the reliability of requests made to the PyPI registry.

Description of changes

  • Replaced the dynamic URL construction with a hardcoded PyPI project URL to avoid issues with incorrect joins and ensure consistent access

Related issues

Checklist

  • I have reviewed the contribution guide.
  • My PR title and commits follow the Conventional Commits convention.
  • My commits include the "Signed-off-by" line.
  • I have signed my commits following the instructions provided by GitHub. Note that we run GitHub's commit verification tool to check the commit signatures. A green verified label should appear next to all of your commits on GitHub.
  • I have updated the relevant documentation, if applicable.
  • I have tested my changes and verified they work as expected.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jun 3, 2025
@AmineRaouane AmineRaouane force-pushed the improve_pypi_registry branch from 7401155 to 7e41b96 Compare June 3, 2025 20:53
@@ -244,11 +244,10 @@ def get_package_page(self, package_name: str) -> str | None:
str | None
The package main page.
"""
url = os.path.join(self.registry_url, "project", package_name)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure why these changes resolve the issue? I tried printing url = os.path.join(self.registry_url, "project", package_name) and it gives the same output as the hardcoded result you've replaced it with. It also seems that response.content.decode("utf-8") and response.text seem to return the same results? Have you been able to avoid the request blocking with this? When I try it I'm still getting blocked.

Copy link
Member

Choose a reason for hiding this comment

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

I can confirm that f"https://pypi.org/project/{package_name}/" works. It seems that the trailing / is essential. Without it, PyPI executes a JavaScript code that sends a POST request and then redirects to the URL with the trailing / appended, and Macaron is not able to follow the POST request.

@@ -244,11 +244,10 @@ def get_package_page(self, package_name: str) -> str | None:
str | None
The package main page.
"""
url = os.path.join(self.registry_url, "project", package_name)
Copy link
Member

Choose a reason for hiding this comment

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

I can confirm that f"https://pypi.org/project/{package_name}/" works. It seems that the trailing / is essential. Without it, PyPI executes a JavaScript code that sends a POST request and then redirects to the URL with the trailing / appended, and Macaron is not able to follow the POST request.

@@ -244,11 +244,10 @@ def get_package_page(self, package_name: str) -> str | None:
str | None
The package main page.
"""
url = os.path.join(self.registry_url, "project", package_name)
url = f"https://pypi.org/project/{package_name}/"
Copy link
Member

Choose a reason for hiding this comment

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

@AmineRaouane Please add a comment above this line to emphasize why / at the end is important.

response = send_get_http_raw(url)
if response:
html_snippets = response.content.decode("utf-8")
return html_snippets
return response.text
Copy link
Member

Choose a reason for hiding this comment

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

This change is not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants