search

SUSE / hanadb_exporter

Prometheus exporter for SAP HANA databases
Apache License 2.0
46 stars 27 forks source link

Update db_manager.py #90

Closed karolyczovek closed 3 years ago

karolyczovek commented 3 years ago

This patch adds exception handling when not all tenant dbs have the monitoring user enabled.

karolyczovek commented 3 years ago

The current UT already cover the functional test, I assume it was always tested with fully configured tenantdb setup

karolyczovek commented 3 years ago

Is a forced push with squashed commits fine for you, or shall I open a new PR with?

arbulu89 commented 3 years ago

Is a forced push with squashed commits fine for you, or shall I open a new PR with?

Yes, good enough

arbulu89 commented 3 years ago

The current UT already cover the functional test, I assume it was always tested with fully configured tenantdb setup

I mean that we don't test the scenario where the exception is raised, and the tenant with the error is not added to the connectors list. It is pretty basic test, but it is not covered now. We would simply need to add a new test, really similar than: https://github.com/SUSE/hanadb_exporter/blob/master/tests/db_manager_test.py#L59

For example, the mock_conn3 would raise this exception that we now catch, and finally the _db_connectors list wouldn't have this element.

Could you add this test? Otherwise I can add it to this PR

karolyczovek commented 3 years ago

Could you add this test? Otherwise I can add it to this PR

Please add it, i am buried deeply into beloved Hana

arbulu89 commented 3 years ago

Merged. Thanks @karolyczovek