OpenSTEF / openstef-dbc

Provides (company specific) database connector for the OpenSTEF package
Mozilla Public License 2.0
1 stars 7 forks source link

Feature KTPS-1550 add get predictors function #8

Closed bremme closed 2 years ago

bremme commented 2 years ago

Voor de KTP measurement forecaster en wellicht in algemene zin is het handig om het ophalen van de load en predictors ook los te kunnen doen. Deze PR voegt functionaliteit toe waarmee de predictors los opgehaald kunnen worden.

Kijk met name even naar:

Laat maar horen of de gekozen implementatie nog beter kan en of als er onbedoelt nog fouten in zitten?

Verder kreeg ik het idee dat er geen gas prijzen waren kan dat kloppen? Misschien meer in het algemeen welke predictor data mag/kan allemaal missen en wat doen we dan. Moet ik bijvoorbeeld allemaal checken en zorgen dat de code nooit crashed bijv?

This should have been written in English perhaps? Let me know if this is an issue so I can translate it.

bremme commented 2 years ago

I noticed the test coverage is quite low. Do we need to address this before merging?

I noticed this as well. It's a bit misleading I guess. These function had no unit test previously, but because I refactored a couple parts all files now count completely as new code. I tried to add a couple basic tests. But its not that straightforward. You either have to mock a every function that makes a database call (most of them). Or you have to mock every call to the database. I agree that ideally this should be tested, but I rather make that a separate task. Perhaps we even need to rethink our architecture to make it better testable in that case.

sonarcloud[bot] commented 2 years ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

37.2% 37.2% Coverage
0.0% 0.0% Duplication