I was playing around with permissions and got an error 400. This is the output of the exception message:
requests.exceptions.HTTPError: 400 Client Error: for url: https://myartifactory/artifactory/api/v2/security/permissions/all-test
400 is not much information about what went wrong, so I dug inside your code and the request library. Finally, I printed the correct variable and found that Artifactory returned this content:
{
"errors" : [ {
"status" : 400,
"message" : "The user: \'username\'\' has admin privileges, and cannot be added to a Permission Target."
} ]
}
This was a helpful error message :) (Although I disagree on the decision behind it).
Ignoring the error message returned by Artifactory in JSON seems not like a good idea to me. It would be better if the error message is also put into the exception which is thrown.
Currently I am wrapping calls to your library with my own exception handler to achieve this:
def get_errors_from_json_response(response: requests.Response) -> Optional[List[str]]:
response_str: bytes = response.content
try:
response_dict: dict = json.loads(response_str)
except:
return None
if not response_dict or not "errors" in response_dict:
return None
return [ f"{error['status']}: {error['message']}" for error in response_dict["errors"] ]
@contextlib.contextmanager
def exception_message(msg):
# from https://stackoverflow.com/questions/17677680/how-can-i-add-context-to-an-exception-in-python
def raise_with_new_msg(ex: Exception, new_msg: str):
ex.args = (new_msg,) + ex.args[1:]
raise
def raise_and_add_message(ex: Exception, new_msg):
msg = '{}: {}'.format(new_msg, ex.args[0]) if ex.args else str(new_msg)
raise_with_new_msg(ex, msg)
try:
yield
except HTTPError as ex:
errors = get_errors_from_json_response(ex.response)
if not errors:
raise_and_add_message(msg)
raise_with_new_msg(ex, f"{msg}: {'; '.join(errors)}")
except Exception as ex:
raise_and_add_message(ex, msg)
Of course, catching exceptions, modifying and re-throwing them is not the best idea. But otherwise I do not get the response object.
On the other hand, you could implement something like raise_for_status() yourself in _generic_http_method_request(), check if the response contains some json with errors and put that into the message, and otherwise use the default implementation:
response: Response = http_method(
f"{self._artifactory.url}/{route}",
auth=self._auth,
**kwargs,
verify=self._verify,
cert=self._cert,
)
if raise_for_status and response.status_code >= 400:
errors = get_errors_from_json_response(response)
if errors:
raise ArtifactoryError("; ".join(errors))
else:
response.raise_for_status()
I was playing around with permissions and got an error 400. This is the output of the exception message:
400is not much information about what went wrong, so I dug inside your code and the request library. Finally, I printed the correct variable and found that Artifactory returned this content:This was a helpful error message :) (Although I disagree on the decision behind it).
Ignoring the error message returned by Artifactory in JSON seems not like a good idea to me. It would be better if the error message is also put into the exception which is thrown.
Currently I am wrapping calls to your library with my own exception handler to achieve this:
Of course, catching exceptions, modifying and re-throwing them is not the best idea. But otherwise I do not get the response object.
On the other hand, you could implement something like
raise_for_status()yourself in_generic_http_method_request(), check if the response contains some json with errors and put that into the message, and otherwise use the default implementation: