Catch Timeout on test connection

[Glutexo]

All Pull Requests:

Check all that apply:

• Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
• Is this PR to correct an issue?
• Is this PR an enhancement?

Complete Description of Additions/Changes:

The --test-connection command only used to catch ConnectionError, including ConnectionTimeout, but not other timeouts such as ReadTimeoutError. Re-used the existing REQUEST_FAILED_EXCEPTIONS list to process all timeouts in the same way.

There is an integration test pull request RedHatInsights/insights-client#244 for this fix.

Card IDs:

• CCT-506
• CCT-724

[Glutexo]

I improved the tests by raising actual exceptions: SSLError (a non-Timeout ConnectionError), ConnectionTimeout (both a Timeout and a ConnectionError) and ReadTimeout (a non-ConnectionError Timeout) rather than the abstract ancestors ConnectionError and Timeout that are caught in the code.

Rebased on current master.

I see the python2x tests are failing. I believed we dropped suppport for Python 2. I will fix the unsupported syntax.

[Glutexo]

Fixed the unsupported Python 2 syntax – an f-string. Rebased on the current master.

[Glutexo]

Re-requesting a review. @m-horky might be the right guy for it

[m-horky]

Generally fine.

When I set the timeout to even lower value (0.01), the output is different -- it is more verbose and the exception is printed twice. Would a fix of that be in scope of this change?

$ insights-client --test-connection
Running Connection Tests...
=== Begin Upload URL Connection Test ===
Testing https://cert.cloud.redhat.com/api/ingress/v1/upload
POST https://cert.cloud.redhat.com/api/ingress/v1/upload
Could not successfully connect to: https://cert.cloud.redhat.com/api/ingress/v1/upload
HTTPSConnectionPool(host='cert.cloud.redhat.com', port=443): Max retries exceeded with url: /api/ingress/v1/upload (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f4fd4550ad0>: Failed to establish a new connection: [Errno 101] Network is unreachable'))
HTTPSConnectionPool(host='cert.cloud.redhat.com', port=443): Max retries exceeded with url: /api/ingress/v1/upload (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f4fd4550ad0>: Failed to establish a new connection: [Errno 101] Network is unreachable'))
Connectivity test failed! Please check your network configuration
Additional information may be in /var/log/insights-client/insights-client.log

[Glutexo]

I found the culprit for the exception being printed twice:

There is another try-except block in the _test_urls function, that only catches ConnectionError (not Timeout). There, the exception is printed and then re-raised. This is completely redundant, as _test_urls is only used in test_connection, which has its own error handling.

I think this can go into this pull request. Will remove.

[xiangce]

Hi @Glutexo - Sorry for bringing this trouble to you, as we just resolved an urgent bug within the repo. Please rebase the master branch of your fork of insights-core and then rebase this branch again. Please reach out to me for any problems when doing the rebase. Apologize again.

[Glutexo]

@xiangce Thank you for reminding me! I fixed everything. However, I accidentally deleted my original remote branch before rebasing. As a result, this pull request got closed and it’s not possible to re-open it even after a new push. I‘ll open a new one.