openclimatefix / pv-site-production

Production service for PV site level forecasts
MIT License
1 stars 3 forks source link

Add a new docker that is responsible to delete old data #49

Closed simlmx closed 1 year ago

simlmx commented 1 year ago

Add a new docker to archive (although in this PR we only delete it) old forecast data.

Changes

Still TODO (should be fixed in THIS PR)

Still TODO (should be done in subsequent PRs):

codecov[bot] commented 1 year ago

Codecov Report

Merging #49 (e7d2aaf) into main (9189be7) will increase coverage by 0.47%. The diff coverage is 92.53%.

:exclamation: Current head e7d2aaf differs from pull request most recent head 08f30ed. Consider uploading reports for the commit 08f30ed to get more accurate results

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   93.84%   94.32%   +0.47%     
==========================================
  Files           9        9              
  Lines         260      317      +57     
==========================================
+ Hits          244      299      +55     
- Misses         16       18       +2     
Impacted Files Coverage Δ
...ference/forecast_inference/data/pv_data_sources.py 94.04% <ø> (ø)
...rence/forecast_inference/models/cos/intensities.py 100.00% <ø> (ø)
...ecast-inference/forecast_inference/utils/config.py 95.00% <ø> (ø)
...cast-inference/forecast_inference/utils/imports.py 91.66% <ø> (ø)
...st-inference/forecast_inference/utils/profiling.py 100.00% <ø> (ø)
database-cleanup/database_cleanup/app.py 91.66% <91.66%> (ø)
forecast-inference/forecast_inference/app.py 92.59% <100.00%> (ø)
...ference/forecast_inference/models/cos/cos_model.py 100.00% <100.00%> (ø)
...orecast-inference/forecast_inference/models/psp.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

peterdudfield commented 1 year ago

Looks good Simon.

Ive left a few minor comments.

I agree with your TODOs. Naming could be

I'm not sure if you should get this through and then adapt it later to save forecast in s3. I think that is more iterrative, but runs the of losing some forecast data. I think thats ok though