-
Notifications
You must be signed in to change notification settings - Fork 6
Handling no received data from WeatherSource #1145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
529bf23
f72bb75
9447e8b
e0828af
6c4c0f8
7502d97
743e99a
74f4816
8e8bed0
21a05a3
e0bcfb6
cee6660
8ed3f29
f7a21d4
646eaa8
e666ea0
171610b
08a2679
2a4d3b7
7d449ee
c83ca93
34d78aa
19d5c3b
ad8946f
e1ec1db
703e581
cd5354d
6723cba
23b732f
88e1ec7
cc9e1a0
60ef7f5
57ca9ab
69838ed
a8f892d
01146b9
955239e
faa36cf
22ca7f6
24441c9
8c2d1f7
9915bee
5dc0f4a
b9d747f
68ea181
c2117e2
b8ff373
35a497d
3951355
24ff4b5
e8a4fee
5cc104b
9f97b00
4be6590
f20db54
fd0823b
ef08123
cf8adb3
701d7e2
b5d2ccf
dfb49df
c75b9fa
dc35434
24e8d3f
0800310
a2911b0
ce294c5
c38e7a5
3ebda49
bdd0ae8
7f6a59c
183e0c8
bf86b78
67000bd
1be0b8e
4b81cd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/* | ||
* © 2024. TU Dortmund University, | ||
* Institute of Energy Systems, Energy Efficiency and Energy Economics, | ||
* Research group Distribution grid planning and operation | ||
*/ | ||
package edu.ie3.datamodel.exceptions; | ||
|
||
/** | ||
* Exception that should be used whenever no data is received{@link | ||
* edu.ie3.datamodel.io.source.DataSource} | ||
*/ | ||
public class NoDataException extends Exception { | ||
|
||
public NoDataException(final String message) { | ||
super(message); | ||
} | ||
|
||
public NoDataException(final String message, final Throwable cause) { | ||
super(message, cause); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
import com.couchbase.client.java.json.JsonObject; | ||
import com.couchbase.client.java.kv.GetResult; | ||
import com.couchbase.client.java.query.QueryResult; | ||
import edu.ie3.datamodel.exceptions.NoDataException; | ||
import edu.ie3.datamodel.io.connectors.CouchbaseConnector; | ||
import edu.ie3.datamodel.io.factory.timeseries.TimeBasedWeatherValueData; | ||
import edu.ie3.datamodel.io.factory.timeseries.TimeBasedWeatherValueFactory; | ||
|
@@ -105,7 +106,7 @@ public Optional<Set<String>> getSourceFields() { | |
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval) { | ||
ClosedInterval<ZonedDateTime> timeInterval) throws NoDataException { | ||
logger.warn( | ||
"By not providing coordinates you are forcing couchbase to check all possible coordinates one by one." | ||
+ " This is not very performant. Please consider providing specific coordinates instead."); | ||
|
@@ -114,7 +115,18 @@ public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | |
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) { | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) | ||
throws NoDataException { | ||
|
||
List<Point> invalidCoordinates = | ||
coordinates.stream() | ||
.filter(coordinate -> idCoordinateSource.getId(coordinate).isEmpty()) | ||
.toList(); | ||
|
||
if (!invalidCoordinates.isEmpty()) { | ||
throw new NoDataException("No data for given coordinates: " + invalidCoordinates); | ||
} | ||
Comment on lines
+126
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. I think there's a way to heal this instead of breaking right away. |
||
|
||
HashMap<Point, IndividualTimeSeries<WeatherValue>> coordinateToTimeSeries = new HashMap<>(); | ||
for (Point coordinate : coordinates) { | ||
Optional<Integer> coordinateId = idCoordinateSource.getId(coordinate); | ||
|
@@ -126,7 +138,8 @@ public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | |
try { | ||
jsonWeatherInputs = queryResult.rowsAsObject(); | ||
} catch (DecodingFailureException ex) { | ||
logger.error("Querying weather inputs failed!", ex); | ||
throw new NoDataException( | ||
"Failed to decode weather data for coordinate " + coordinate, ex); | ||
} | ||
if (jsonWeatherInputs != null && !jsonWeatherInputs.isEmpty()) { | ||
Set<TimeBasedValue<WeatherValue>> weatherInputs = | ||
|
@@ -138,32 +151,52 @@ public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | |
new IndividualTimeSeries<>(weatherInputs); | ||
coordinateToTimeSeries.put(coordinate, weatherTimeSeries); | ||
} | ||
} else logger.warn("Unable to match coordinate {} to a coordinate ID", coordinate); | ||
} | ||
} | ||
return coordinateToTimeSeries; | ||
} | ||
|
||
@Override | ||
public Optional<TimeBasedValue<WeatherValue>> getWeather(ZonedDateTime date, Point coordinate) { | ||
public TimeBasedValue<WeatherValue> getWeather(ZonedDateTime date, Point coordinate) | ||
throws NoDataException { | ||
Optional<Integer> coordinateId = idCoordinateSource.getId(coordinate); | ||
if (coordinateId.isEmpty()) { | ||
logger.warn("Unable to match coordinate {} to a coordinate ID", coordinate); | ||
return Optional.empty(); | ||
logger.error("Unable to match coordinate {} to a coordinate ID", coordinate); | ||
throw new NoDataException("No coordinate ID found for the given point: " + coordinate); | ||
Comment on lines
+164
to
+165
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logging an error is obsolete when we also throw and exception in the next line, imho |
||
} | ||
try { | ||
CompletableFuture<GetResult> futureResult = | ||
connector.get(generateWeatherKey(date, coordinateId.get())); | ||
GetResult getResult = futureResult.join(); | ||
JsonObject jsonWeatherInput = getResult.contentAsObject(); | ||
return toTimeBasedWeatherValue(jsonWeatherInput); | ||
return toTimeBasedWeatherValue(jsonWeatherInput) | ||
.orElseThrow( | ||
() -> | ||
new NoDataException( | ||
"No valid weather data found for the given date and coordinate.")); | ||
Comment on lines
+175
to
+176
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here: Maybe try to take an earlier data point instead? |
||
} catch (DecodingFailureException ex) { | ||
logger.error("Decoding to TimeBasedWeatherValue failed!", ex); | ||
return Optional.empty(); | ||
throw new NoDataException( | ||
"Failed to decode weather data for coordinate " + coordinate + " and date " + date, ex); | ||
} catch (DocumentNotFoundException ex) { | ||
return Optional.empty(); | ||
throw new NoDataException( | ||
"Weather document not found for coordinate " + coordinate + " and date " + date, ex); | ||
} catch (CompletionException ex) { | ||
if (ex.getCause() instanceof DocumentNotFoundException) return Optional.empty(); | ||
else throw ex; | ||
Throwable cause = ex.getCause(); | ||
if (cause instanceof DocumentNotFoundException) { | ||
throw new NoDataException( | ||
"Weather document not found in completion stage for coordinate " | ||
+ coordinate | ||
+ " and date " | ||
+ date, | ||
cause); | ||
} else { | ||
throw new NoDataException( | ||
"Unexpected completion exception while retrieving weather data for coordinate " | ||
+ coordinate | ||
+ " and date " | ||
+ date, | ||
ex); | ||
} | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
import static edu.ie3.datamodel.utils.validation.UniquenessValidationUtils.checkWeatherUniqueness; | ||
|
||
import edu.ie3.datamodel.exceptions.DuplicateEntitiesException; | ||
import edu.ie3.datamodel.exceptions.NoDataException; | ||
import edu.ie3.datamodel.exceptions.SourceException; | ||
import edu.ie3.datamodel.exceptions.ValidationException; | ||
import edu.ie3.datamodel.io.connectors.CsvFileConnector; | ||
|
@@ -90,25 +91,63 @@ public Optional<Set<String>> getSourceFields() { | |
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval) { | ||
return trimMapToInterval(coordinateToTimeSeries, timeInterval); | ||
ClosedInterval<ZonedDateTime> timeInterval) throws NoDataException { | ||
|
||
Map<Point, IndividualTimeSeries<WeatherValue>> result = | ||
trimMapToInterval(coordinateToTimeSeries, timeInterval); | ||
|
||
if (result == null || result.isEmpty()) { | ||
throw new NoDataException( | ||
"No weather data found for the given time interval: " + timeInterval); | ||
} | ||
|
||
return result; | ||
} | ||
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) { | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) | ||
throws NoDataException { | ||
|
||
List<Point> invalidCoordinates = | ||
coordinates.stream() | ||
.filter(coordinate -> !coordinateToTimeSeries.containsKey(coordinate)) | ||
.toList(); | ||
|
||
if (!invalidCoordinates.isEmpty()) { | ||
throw new NoDataException("No data for given coordinates: " + invalidCoordinates); | ||
} | ||
Comment on lines
+117
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
|
||
Map<Point, IndividualTimeSeries<WeatherValue>> filteredMap = | ||
coordinateToTimeSeries.entrySet().stream() | ||
.filter(entry -> coordinates.contains(entry.getKey())) | ||
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); | ||
return trimMapToInterval(filteredMap, timeInterval); | ||
|
||
Map<Point, IndividualTimeSeries<WeatherValue>> result = | ||
trimMapToInterval(filteredMap, timeInterval); | ||
|
||
if (result == null || result.isEmpty()) { | ||
throw new NoDataException( | ||
"No weather data found for the given time interval: " + timeInterval); | ||
} | ||
return result; | ||
} | ||
|
||
@Override | ||
public Optional<TimeBasedValue<WeatherValue>> getWeather(ZonedDateTime date, Point coordinate) { | ||
public TimeBasedValue<WeatherValue> getWeather(ZonedDateTime date, Point coordinate) | ||
throws NoDataException { | ||
IndividualTimeSeries<WeatherValue> timeSeries = coordinateToTimeSeries.get(coordinate); | ||
if (timeSeries == null) return Optional.empty(); | ||
return timeSeries.getTimeBasedValue(date); | ||
|
||
if (timeSeries == null) { | ||
throw new NoDataException("No weather data found for the given coordinate: " + coordinate); | ||
} | ||
|
||
return timeSeries | ||
.getTimeBasedValue(date) | ||
.orElseThrow( | ||
() -> | ||
new NoDataException( | ||
"No weather data found for the given coordinate: " + coordinate)); | ||
Comment on lines
+149
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
*/ | ||
package edu.ie3.datamodel.io.source.influxdb; | ||
|
||
import edu.ie3.datamodel.exceptions.NoDataException; | ||
import edu.ie3.datamodel.io.connectors.InfluxDbConnector; | ||
import edu.ie3.datamodel.io.factory.timeseries.TimeBasedWeatherValueData; | ||
import edu.ie3.datamodel.io.factory.timeseries.TimeBasedWeatherValueFactory; | ||
|
@@ -59,14 +60,18 @@ public Optional<Set<String>> getSourceFields() { | |
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval) { | ||
ClosedInterval<ZonedDateTime> timeInterval) throws NoDataException { | ||
try (InfluxDB session = connector.getSession()) { | ||
String query = createQueryStringForTimeInterval(timeInterval); | ||
QueryResult queryResult = session.query(new Query(query)); | ||
Stream<Optional<TimeBasedValue<WeatherValue>>> optValues = | ||
optTimeBasedValueStream(queryResult); | ||
Set<TimeBasedValue<WeatherValue>> timeBasedValues = | ||
filterEmptyOptionals(optValues).collect(Collectors.toSet()); | ||
if (timeBasedValues.isEmpty()) { | ||
throw new NoDataException( | ||
"No weather data found for the given time interval: " + timeInterval); | ||
} | ||
Map<Point, Set<TimeBasedValue<WeatherValue>>> coordinateToValues = | ||
timeBasedValues.stream() | ||
.collect( | ||
|
@@ -81,10 +86,22 @@ public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | |
|
||
@Override | ||
public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) { | ||
ClosedInterval<ZonedDateTime> timeInterval, Collection<Point> coordinates) | ||
throws NoDataException { | ||
if (coordinates == null) return getWeather(timeInterval); | ||
Map<Point, Optional<Integer>> coordinatesToId = | ||
coordinates.stream().collect(Collectors.toMap(point -> point, idCoordinateSource::getId)); | ||
|
||
List<Point> invalidCoordinates = | ||
coordinatesToId.entrySet().stream() | ||
.filter(entry -> entry.getValue().isEmpty()) | ||
.map(Map.Entry::getKey) | ||
.toList(); | ||
|
||
if (!invalidCoordinates.isEmpty()) { | ||
throw new NoDataException("No data for given coordinates: " + invalidCoordinates); | ||
} | ||
|
||
HashMap<Point, IndividualTimeSeries<WeatherValue>> coordinateToTimeSeries = new HashMap<>(); | ||
try (InfluxDB session = connector.getSession()) { | ||
for (Map.Entry<Point, Optional<Integer>> entry : coordinatesToId.entrySet()) { | ||
|
@@ -97,25 +114,37 @@ public Map<Point, IndividualTimeSeries<WeatherValue>> getWeather( | |
optTimeBasedValueStream(queryResult); | ||
Set<TimeBasedValue<WeatherValue>> timeBasedValues = | ||
filterEmptyOptionals(optValues).collect(Collectors.toSet()); | ||
IndividualTimeSeries<WeatherValue> timeSeries = | ||
new IndividualTimeSeries<>(timeBasedValues); | ||
coordinateToTimeSeries.put(entry.getKey(), timeSeries); | ||
if (!timeBasedValues.isEmpty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it also lead to an exception in case timeBasedValues is empty, since one could assume we would expect weather data when asking for it and those aren't there? Not sure about this, I might have to rethink on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, testing this would also be great. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested throwing an exception when timeBasedValues is empty, but that broke existing behavior and tests. For now we keep it simple: invalid coordinates throw a NoDataException, but valid coordinates with no data just return an empty series. Or do we really want it that strict? If so we could use my last commit which I reverted again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be great to have equivalent behavior across types of data sources, at least where it's possible |
||
IndividualTimeSeries<WeatherValue> timeSeries = | ||
new IndividualTimeSeries<>(timeBasedValues); | ||
coordinateToTimeSeries.put(entry.getKey(), timeSeries); | ||
} | ||
} | ||
} | ||
} | ||
return coordinateToTimeSeries; | ||
} | ||
|
||
@Override | ||
public Optional<TimeBasedValue<WeatherValue>> getWeather(ZonedDateTime date, Point coordinate) { | ||
public TimeBasedValue<WeatherValue> getWeather(ZonedDateTime date, Point coordinate) | ||
throws NoDataException { | ||
Optional<Integer> coordinateId = idCoordinateSource.getId(coordinate); | ||
if (coordinateId.isEmpty()) { | ||
return Optional.empty(); | ||
throw new NoDataException("No coordinate ID found for the given point: " + coordinate); | ||
} | ||
|
||
try (InfluxDB session = connector.getSession()) { | ||
String query = createQueryStringForCoordinateAndTime(date, coordinateId.get()); | ||
QueryResult queryResult = session.query(new Query(query)); | ||
return filterEmptyOptionals(optTimeBasedValueStream(queryResult)).findFirst(); | ||
return filterEmptyOptionals(optTimeBasedValueStream(queryResult)) | ||
.findFirst() | ||
.orElseThrow( | ||
() -> | ||
new NoDataException( | ||
"No weather data available for the given date " | ||
+ date | ||
+ " and coordinate " | ||
+ coordinate)); | ||
} | ||
} | ||
|
||
|
@@ -167,10 +196,10 @@ public List<ZonedDateTime> getTimeKeysAfter(ZonedDateTime time, Point coordinate | |
* @return weather data for the specified time and coordinate | ||
*/ | ||
public IndividualTimeSeries<WeatherValue> getWeather( | ||
ClosedInterval<ZonedDateTime> timeInterval, Point coordinate) { | ||
ClosedInterval<ZonedDateTime> timeInterval, Point coordinate) throws NoDataException { | ||
Optional<Integer> coordinateId = idCoordinateSource.getId(coordinate); | ||
if (coordinateId.isEmpty()) { | ||
return new IndividualTimeSeries<>(UUID.randomUUID(), Collections.emptySet()); | ||
throw new NoDataException("No data for given coordinates: " + coordinate); | ||
} | ||
try (InfluxDB session = connector.getSession()) { | ||
String query = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to the end of this subsection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder to move this to the
Unreleased/Snapshot
section