SolarArbiter / solarforecastarbiter-api

HTTP API and database schema for the Solar Forecast Arbiter
https://api.solarforecastarbiter.org
MIT License
10 stars 6 forks source link

Improve report validation #193

Closed lboeman closed 3 years ago

lboeman commented 4 years ago

There is no validation for requirements such as fx.interval_length < obs.interval_length in the API. This validation is performed in the DataModel and results in a value error thrown in the worker while it attempts to compute the report. From a user perspective, this just results in a failed report with no information about why the failure occurred.

This issue presents itself directly when running locally with USE_FAKE_REDIS = TRUE because the computation is run immediately instead of being handed off to a worker, resulting in a 500.

The dashboard should definitely prevent this in the first place, but it should probably be handled elsewhere as well. I see there being two ways to handle this:

@alorenzo175, @wholmgren Do you guys have any thoughts about this? I think this is relatively minor, as the dashboard can help avoid these issues, but it's probably something we should handle eventually.

alorenzo175 commented 4 years ago

We should do both. I think the api should validate the report before storing. I also think we should be able to store and display any errors that occur when computing the report.

Lets start with validating the report on upload and think more about how to store and display a failure during compute.

alorenzo175 commented 4 years ago

I think storing/displaying report errors was handled by the report refactor. We do need a little more validation, for example, verifying that the units match