remove explicit 404 message check from createException#3180
remove explicit 404 message check from createException#3180coltonb-arcadia wants to merge 1 commit intoPyGithub:mainfrom coltonb-arcadia:do-not-key-404-off-message
Conversation
|
we are experiencing the same issue the message was changed from "not found" to |
|
I'm not a maintainer myself but I did notice there is a test that specifies a message. Might be worth updating to better match what the API responds with now. |
|
I believe that you can't simply remove the message, because the library tries to intercept and help the developer with various types of 404. I think it's important to include a treatment of: "No object found for the path ". If you do this directly in requests it is bad, we need this change in get contents. |
👍
This might be a better design as it keeps the handling for the specific exception closer to the code handling the response for that api endpoint. But, right now requester captures all 4XX responses in its global handling, and unwinding that seems like a lot. I opened another PR that keeps the old check, and adds a new clause for the new message. Thus we preserve old behaviors without rearchitecting the 4XX handling. Also updated unit tests. |
Alternative to #3180 that fixes #3179, but tries to preserve existing behavior. UnknownObjectException is raised for all kinds of calls that may return a 404, for example [get_repo](https://pygithub.readthedocs.io/en/stable/github_objects/Organization.html#github.Organization.Organization.get_repo). Those API calls still return a "Not Found" message: ```python >>> from github import Github >>> from os import environ >>> Github(environ['GITHUB_TOKEN']).get_organization('asdfqwertyjkl') Traceback (most recent call last): File "<python-input-3>", line 1, in <module> Github(environ['GITHUB_TOKEN']).get_organization('asdfqwertyjkl') ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^ File ".../venv/lib/python3.13/site-packages/github/MainClass.py", line 408, in get_organization headers, data = self.__requester.requestJsonAndCheck("GET", f"/orgs/{login}") ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../venv/lib/python3.13/site-packages/github/Requester.py", line 586, in requestJsonAndCheck return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url))) ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File ".../venv/lib/python3.13/site-packages/github/Requester.py", line 744, in __check raise self.createException(status, responseHeaders, data) github.GithubException.UnknownObjectException: 404 {"message": "Not Found", "documentation_url": "https://docs.github.com/rest/orgs/orgs#get-an-organization", "status": "404"} ``` So, in this PR we honor both kinds of messages, but still no _other_ kinds of messages. Tests are updated to reflect the new case.
|
Merged #3181 in favour of this one. |
Addresses #3179
Not sure if test updates are required for this or where to make them.