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

Query objects.inv from homepage url #40

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

mathbou
Copy link
Contributor

@mathbou mathbou commented Jan 31, 2022

Problem

On PyPi, it's very common that the 'Homepage' url links to the documentation.

Proposal

Add the homepage URL to the project links

Copy link
Member

@domdfcoding domdfcoding left a comment

Choose a reason for hiding this comment

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

I'm OK with the changes in principle, I just have a few comments.

seed_intersphinx_mapping/__init__.py Outdated Show resolved Hide resolved
seed_intersphinx_mapping/__init__.py Outdated Show resolved Hide resolved
seed_intersphinx_mapping/__init__.py Show resolved Hide resolved
raise ValueError(f"objects.inv not found at url {objects_inv_url}: HTTP Status {r.status_code}.")

return docs_url
stderr_writer(f"WARNING: objects.inv not found at url {objects_inv_url}: HTTP Status {r.status_code}.")
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 reluctant to accept this change as get_sphinx_doc_url is a public function that other modules can use to query documentation URLs. I'd like the caller to still be able to catch the ValueError and display the message to the user themselves.

If you're wanting to make the warning on line 197 more specific -- to say why the documentation URL couldn't be obtained -- how about including the exception message text? E.g.:

except (ValueError, requests.exceptions.ConnectionError, requests.exceptions.Timeout) as e:
	# Couldn't get it from PyPI, trying fallback mapping
	if project_name in fallback_mapping():
		doc_url = fallback_mapping()[project_name]
		intersphinx_mapping[project_name] = (doc_url, None)
	else:
        stderr_writer(f"WARNING: Unable to determine documentation url for project {project_name}: {str(e)}")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand your point, removing the raise was here to ensure all URLs are queried until one is good.
Due to the addition of the home-page URL in docs_urls, they may be more than one URL to iterate over. Keeping the raise makes get_sphinx_doc_url always return or fail on the first one, so if the documentation URL is bad, but homepage is good, it will not work.

@repo-helper repo-helper bot added failure: docs The docs check is failing. failure: Linux The Linux tests are failing. failure: Windows The Windows tests are failing. failure: mypy The mypy check is failing. labels Feb 1, 2022
@repo-helper repo-helper bot added failure: Linux The Linux tests are failing. and removed failure: docs The docs check is failing. failure: Linux The Linux tests are failing. failure: Windows The Windows tests are failing. labels Mar 10, 2022
@coveralls
Copy link

coveralls commented Mar 15, 2022

Pull Request Test Coverage Report for Build 2098826777

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 27 of 27 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 97.122%

Totals Coverage Status
Change from base Build 2098523635: 0.2%
Covered Lines: 135
Relevant Lines: 139

💛 - Coveralls

@repo-helper repo-helper bot added failure: Windows The Windows tests are failing. failure: Linux The Linux tests are failing. and removed failure: mypy The mypy check is failing. failure: Linux The Linux tests are failing. failure: Windows The Windows tests are failing. labels Mar 15, 2022
@repo-helper repo-helper bot removed failure: Linux The Linux tests are failing. failure: Windows The Windows tests are failing. labels Apr 5, 2022
@stale stale bot added the stale label Oct 8, 2022
@stale stale bot removed the stale label Nov 17, 2022
@stale stale bot added the stale label May 18, 2023
@stale stale bot removed the stale label May 18, 2024
@repo-helper repo-helper bot added the failure: Windows The Windows tests are failing. label May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failure: Windows The Windows tests are failing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants