Willmish
commented
1 year ago I have mixed opinions on this, so will share both reasons why I think we might want it and why not. Why we SHOULD throw an exception:
- we can catch the exception higher up and return a 400 error on invalid time range in the endpoints. This immediately gives feedback to the user that there cannot be any valid values returned for that time range. (if not already covered, we can then also return this when startDate is later than endDate)
Why we SHOULDN'T throw an exception:
- if we preprocess the time range to determine whether to throw an exception or not, we need to make assumptions on ALL underlying data sources, that this time range is invalid. And although for WattTime, EM and EMfree this seems to make sense at the moment, maybe we need to keep in mind future datasources which might accept time ranges for future dates on historical endpoints. This can be disregarded however, if we never want to allow forecast data to be returned from a historical endpoint (which does make sense).
Additionally, when implementing this we need to think in which timezone and at what times the cutoff time for most recent emission is - e.g. if EMFree has a few hour delay, then requesting current time emissions might be invalid, as the most recent emission is from few hours ago.
YaSuenag
vaughanknight
github-actions[bot]
danuw
What happened?
We would receive HTTP 204 (No content) when we pass future datetime to endpoints of current/historical data (e.g. /emissions/bylocation) in WebAPI. However these endpoints would not handle forecast data, so future datetime as parameters are not expected.
We've discussed about this whether it would be better to throw exception (e.g. ArgumentOutOfRangeException) in this case in #389 and #384. If throwing exception is appropriate, we need to fix all of datasources (WattTime, ElectricityMaps, ElectricityMapsFree, JSON).
Code of Conduct
Feature Commitment