@gnaponie can you PTAL?

This needs to be removed.

Can we extend that to check also koji? This PR should handle issues incurred with all services needed for retrieving the gating.yaml.
And we should catch also wider exceptions...

Checking the logs I also see sometimes this happens:
werkzeug.exceptions.BadGateway: 502 Bad Gateway: Failed to find Koji build for "perl-Sys-Virt-5.6.0-1.module+el8.1.0+3895+267fb0f" at ...

This needs to be removed.

oops! removing it now

Can we extend that to check also koji? This PR should handle issues incurred with all services needed for retrieving the gating.yaml.
And we should catch also wider exceptions...

Do you mean handling general Exception instead of a specific one while connecting to koji using the xmlrpc client?
I am updating the code, please let me know if that is in line with what you think. Sorry if I am missing something. :)

Checking the logs I also see sometimes this happens:
werkzeug.exceptions.BadGateway: 502 Bad Gateway: Failed to find Koji build for "perl-Sys-Virt-5.6.0-1.module+el8.1.0+3895+267fb0f" at ...

I think we raise this intentionally? Am I missing something?
https://pagure.io/greenwave/blob/master/f/greenwave/resources.py#_137

Do you mean handling general Exception instead of a specific one while connecting to koji using the xmlrpc client?

Yeah, I would maybe catch the known issues and then catch every exception just to be sure.

I think we raise this intentionally? Am I missing something?
https://pagure.io/greenwave/blob/master/f/greenwave/resources.py#_137

Yeah. But I thought the reason of the PR was to log nicely the exceptions.
For example, in this case, the Bad Gateway is handled, but we print the whole traceback even if we know the problem. We should print only one line that says that we couldn't talk to koji.
Otherwise it becomes really hard to read the logs with all those tracebacks.

I think we raise this intentionally? Am I missing something?
https://pagure.io/greenwave/blob/master/f/greenwave/resources.py#_137

Yeah. But I thought the reason of the PR was to log nicely the exceptions.
For example, in this case, the Bad Gateway is handled, but we print the whole traceback even if we know the problem. We should print only one line that says that we couldn't talk to koji.
Otherwise it becomes really hard to read the logs with all those tracebacks.

Oh! I think we should have tracebacks in logs. This is only visible by admins. There's a potential risk of hiding valuable debugging information by omitting them.

I do think that the users shouldn't see these. For them, they should only see a reasonable explanation of the error: "oops! I can't talk to Koji right now".

Instead of wrapping the exception, you could add something like this to json_error:

if isinstance(error, urllib3.exceptions.MaxRetryError):
        current_app.logger.exception('Connection error: {}'.format(error))
        if error.request.url.endswith('gating.yaml'):
            msg = 'There was an error retrieving the gating.yaml file at {}'.format(error.request.url)
        else:
            msg = 'Error connecting to {}'.format(error.request.url)
        status_code = 502

I updated the PR. Can you please check and let me know if this works?

I'll add tests if we agree that this approach is fine. :)

Can this be removed?

Can this be removed?

Looks good to me

Made the changes and added a test. :)

LGTM

Thanks everyone!

merging this.