Hide internal logging exceptions

[FlorianVeaux]

What does this PR do?

If an internal logging exception is raised while using the datadog_checks_downloader, we should not display it to the end user.

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

• Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
• PR title must be written as a CHANGELOG entry (see why)
• Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
• PR must have changelog/ and integration/ labels attached

[ofek]

Can we move the logic to https://github.com/DataDog/integrations-core/blob/master/datadog_checks_downloader/datadog_checks/downloader/cli.py?

[FlorianVeaux]

No, the logic has to happen before: https://github.com/DataDog/integrations-core/blob/master/datadog_checks_downloader/datadog_checks/downloader/__main__.py#L7

And having it in __main__ requires styling tricks, as you need to run code between imports and all the styling tools we use complain about that.

[codecov]

Codecov Report

Merging #5848 into master will decrease coverage by 7.95%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
...s_downloader/datadog_checks/downloader/__init__.py	100% <100%> (ø)
...ts/tooling/configuration/consumers/test_example.py
...checks_dev/tests/plugin/test_environment_runner.py
oracle/tests/test_oracle.py
...r/datadog_checks/openstack_controller/__about__.py
twistlock/datadog_checks/twistlock/twistlock.py
linkerd/datadog_checks/linkerd/linkerd.py
datadog_checks_base/tests/test_tracing.py
gitlab_runner/tests/test_integration.py
...hecks/openstack_controller/openstack_controller.py
... and 848 more

[trishankatdatadog]

Thanks @FlorianVeaux !

So, hold on, I'm a little confused. What is the effect of this? Would this swallow logging.exception() calls?

[FlorianVeaux]

No this only swallows "internal" errors from the logging module.

For example on Python2 this would display an error if you don't set raiseExceptions = False:

import logging
logger = logging.getLogger('foo')

logger.warn("bar")

Python will show the error "No handler could be found for logger 'foo'"

[trishankatdatadog]

No this only swallows "internal" errors from the logging module.

So, sorry, why do we need this in relation to this issue?

[FlorianVeaux]

There are two issues in the securesystemslib library.

The first one is that it shows a warning when colorama is not installed. This issue is fixed with the pull request you mentioned.

The second issue is that the logger is not initialized correctly in that library. It would need to add a "handler" to correctly log messages to stdout/a file/something else...
The result of that is that when the library will try to log something, it won't work and Python will show an error saying that there "No handler could be found..."

This is not an issue on Python3 though as it has a default handler to log on stdout

[trishankatdatadog]

Oh, I see, thanks for the explanation! Hmm, then we need to file another issue for the second bug in securesystemslib, no?

[trishankatdatadog]

Also, would be great if you add a short note to add context around this LoC, if you don't mind terribly much, please...

[FlorianVeaux]

Yes, the issue I opened mentions both problems: secure-systems-lab/securesystemslib#210

But my PR doesn't fix that second part as it requires more knowledge about the library.

Also, would be great if you add a short note to add context around this LoC, if you don't mind terribly much, please...

I think I've given all the details in both the issue and the PR on the securesystemlib repo.
For this PR only, I don't think we should relate the change to the securesystemlib issue. It's a good practice to have raiseExceptions = False in any case for a tool like the downloader which output will be read by humans.