Changed order of fields in InfluxDB queries, repurpose SQl connection #272
Conversation
Signed-off-by: Mia <mia.krause004@stud.fh-dortmund.de>
Codecov Report
@@ Coverage Diff @@
## dev #272 +/- ##
============================================
+ Coverage 78.24% 78.25% +0.01%
- Complexity 1943 1947 +4
============================================
Files 247 247
Lines 7900 7910 +10
Branches 747 747
============================================
+ Hits 6181 6190 +9
Misses 1301 1301
- Partials 418 419 +1
Continue to review full report at Codecov.
|
|
!test |
| .map(weatherValueFactory::get); | ||
| } | ||
|
|
||
| private String createQueryStringForIntervalAndCoordinate( |
There was a problem hiding this comment.
| private String createQueryStringForIntervalAndCoordinate( | |
| private String createQueryStringForCoordinateAndTimeInterval( |
| + createTimeConstraint(timeInterval); | ||
| } | ||
|
|
||
| private String createQueryStringForDateAndCoordinate(ZonedDateTime date, int coordinateId) { |
There was a problem hiding this comment.
| private String createQueryStringForDateAndCoordinate(ZonedDateTime date, int coordinateId) { | |
| private String createQueryStringForCoordinateAndTime(ZonedDateTime date, int coordinateId) { |
| ClosedInterval<ZonedDateTime> timeInterval, int coordinateId) { | ||
| return createQueryStringForInterval(timeInterval) | ||
| return BASIC_QUERY_STRING | ||
| + " where " |
There was a problem hiding this comment.
You might consider introducing a constant for " where ". It's not too vital, but reduces the number of code smells...
| return coordinateToTimeSeries; | ||
| } | ||
|
|
||
| private Connection getConnection() throws SQLException { |
There was a problem hiding this comment.
From my POV we should discuss, if holding a connection should better be moved to the connector itself. Either SqlConnector having an attribute reuseConnection or the getter being like
public getConnection() throws SQLException {
return getConnection(false);
}
public getConnection(boolean reuseConnection) throws SQLException {
if (reuseConnection && (connection == null || connection.isClosed()) || !reuseConnection) connection = connector.getConnection();
return connection;
}What are your thoughts on this?
There was a problem hiding this comment.
Very good point. Especially bc otherwise it would be impossible to execute concurrent db calls with the same weather source (we would have to create a new weather source for each concurrent call). However, I would prefer having a slightly more elaborated approach, where the number of parallel connections is tracked in order to not create too much concurrent connections. This might be possible with a list or map holding the actual open connections and either returns one of them that is currently not used or creates a new one. Furthermore, in this case the connector should keep "its space clean" by automatically remove unused connections whenever a new getConnection call is executed.
Maybe we can discuss this in a short call?
There was a problem hiding this comment.
Boy, that escalated quickly. 😃
I'm open to a call. But then we might consider splitting up the PR into on InfluxDB- and one SQL-PR?!
|
!test |
Signed-off-by: Mia <mia.krause004@stud.fh-dortmund.de>
…nto mk/#271-minor-database-issues
Solves #271