[Feature Contribution]: Better to throw exception when future datetime is specified to endpoints for current/historical data

[YaSuenag]

trafficstars

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

• [X] I agree to follow this project's Code of Conduct

Feature Commitment

• [X] I commit to contributing this feature as a PR and working with the GSF to merge this feature into the Carbon Aware SDK.

[Willmish]

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]

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).

I think we can disallow historical endpoint never accept request for forecasting because it is for "historical".

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.

I've raised this issue about passing future datetime to historical endpoint, so I think we don't need to think about this case.
But I think we can following cases:

1. do not specify time range:
   • SDK can return current emission data because time range does not specified.
2. start time is specified, and emission data in range is available
   • SDK can return emission data.
3. start time is specified, but emission data is out of range
   • SDK cannot return emission data.
4. start time points the future
   • SDK should not return (should throw exception) on historical endpoint.

I think your concern fits case 2 and 3, it means the SDK user specifies start datetime, so I think it can be allowed that the SDK would not return any data when emission data is out of specified time range.

[vaughanknight]

Can we please create an ADR for this as per the meeting last week on the 2023-10-10 #402 . Thanks!

[github-actions[bot]]

This issue has not had any activity in 120 days. Please review this issue and ensure it is still relevant. If no more activity is detected on this issue for the next 20 days, it will be closed automatically.

[YaSuenag]

Wait, do not close this issue.

I will start to work when I can (maybe after clarifying vNext)

[danuw]

What is next on this one please?

[github-actions[bot]]

This issue has not had any activity in 120 days. Please review this issue and ensure it is still relevant. If no more activity is detected on this issue for the next 20 days, it will be closed automatically.

[YaSuenag]

Please remove stale label from this issue.

Priority of this issue is not high, but it is worth to discuss I think. I'll back when other issues/PRs are closed.