fix(resolver): indicate resolver type in error messages #859
Conversation
LalatenduMohanty
force-pushed
the
858
branch
2 times, most recently
from
13998f8 to
eb93e88
Compare
src/fromager/resolver.py
Outdated
|
|
||
| raise resolvelib.resolvers.ResolverException( | ||
| f"found no match for {r}, searching for {file_type_info}, {prerelease_info} pre-release versions, in cache or at {self.sdist_server_url}" | ||
| f"found no match for {r} using {self.get_provider_description()}, searching for {file_type_info}, {prerelease_info} pre-release versions, in cache or at {self.sdist_server_url}" |
There was a problem hiding this comment.
The GitHub and gitlab providers inherit this method, and so are going to inherit this exception and its message. But the content of the message doesn't make sense for those providers, because they don't look at the sdist_server_url. So maybe more of this text should go into the PyPI provider description? Or it should be refactored in some other way to ensure that only the relevant information is included in the error message.
There was a problem hiding this comment.
... then format the description with self.provider_description.format(self=self)
There was a problem hiding this comment.
Tracing the inheritance:
-
BaseProvider._get_no_match_error_message() returns: "found no match for {r} using {self.get_provider_description()}"
-
PyPIProvider overrides it with a PyPI-specific message mentioning sdist_server_url
-
GenericProvider inherits from BaseProvider (doesn't override it)
-
GitHubTagProvider and GitLabTagProvider inherit from GenericProvider
src/fromager/resolver.py
Outdated
| def get_provider_description(self) -> str: | ||
| """Return a human-readable description of the provider type | ||
| This is used in error messages to indicate what resolver was being used. | ||
| """ | ||
| return type(self).__name__ |
There was a problem hiding this comment.
Suggestion: Instead of a callback, add provider_description: typing.ClassVar[str].
src/fromager/resolver.py
Outdated
|
|
||
| raise resolvelib.resolvers.ResolverException( | ||
| f"found no match for {r}, searching for {file_type_info}, {prerelease_info} pre-release versions, in cache or at {self.sdist_server_url}" | ||
| f"found no match for {r} using {self.get_provider_description()}, searching for {file_type_info}, {prerelease_info} pre-release versions, in cache or at {self.sdist_server_url}" |
There was a problem hiding this comment.
... then format the description with self.provider_description.format(self=self)
src/fromager/resolver.py
Outdated
| def get_provider_description(self) -> str: | ||
| return f"GitHub tag resolver (repository: {self.organization}/{self.repo})" | ||
|
|
There was a problem hiding this comment.
| def get_provider_description(self) -> str: | |
| return f"GitHub tag resolver (repository: {self.organization}/{self.repo})" | |
| provider_description = "GitHub tag resolver (repository: {self.organization}/{self.repo})" |
LalatenduMohanty
force-pushed
the
858
branch
3 times, most recently
from
dce7abd to
1143942
Compare
Add provider description to error messages when resolution fails. Previously, errors from custom resolvers (GitHub, GitLab, etc.) were indistinguishable from PyPI resolver errors, making debugging difficult. Closes: python-wheel-build#858 Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
LalatenduMohanty
force-pushed
the
858
branch
from
1143942 to
370c3c8
Compare
|
@tiran I have addressed your comments but have a little bit more to keep the log messages more user friendly. PTAL |
3 participants
Add provider description to error messages when resolution fails. Previously, errors from custom resolvers (GitHub, GitLab, etc.) were indistinguishable from PyPI resolver errors, making debugging difficult.
Closes: #858