openclimatefix / pv-site-api

Site specific API for PV forecasting
6 stars 11 forks source link

dict changes size #186

Open peterdudfield opened 1 month ago

peterdudfield commented 1 month ago

Describe the bug

I think this happens when two fast api threads are trying to clear the cache at the same time

Screenshot 2024-09-04 at 09 05 39

The code is here

Expected behavior

no errors

solution?

michael-gendy-mention-me commented 1 month ago

@peterdudfield I could take a look at this if nobody else has taken it? Could be a good first contribution

peterdudfield commented 3 weeks ago

Let me know how you got on @michael-gendy-mention-me ?

michael-gendy-mention-me commented 2 weeks ago

Apologies @peterdudfield I didn't get notified by your thumbs up - I've only just had a look after your comment. I'll give it a go and feedback in the next few days

michael-gendy-mention-me commented 1 week ago

@peterdudfield Struggling to reproduce this error - think I could be missing something. Trying to add a site locally and call get_pv_estimate_clearsky, which will then call the buggy remove_old_cache method through the cache_response decorator. I'm getting the following error:

  File "/Users/micheal.gendy/Library/Caches/pypoetry/virtualenvs/pv-site-api-JFNXLxhc-py3.12/lib/python3.12/site-packages/fastapi/concurrency.py", line 36, in contextmanager_in_threadpool
    raise e
NameError: name 'connection' is not defined

Do I need to sort anything else out with my local? I'm just running the poetry run uvicorn pv_site_api.main:app --reload, and the dockerfile doesn't seem to be of any help so far also (failing to run the poetry install)

Side note: As a new user of the repo I think it could benefit from some more documentation around setup for prod - took a while to find where to generate credentials to get my access_code and add my own site. Happy to create another PR for some clearer docs for beginners.

peterdudfield commented 1 week ago

@peterdudfield Struggling to reproduce this error - think I could be missing something. Trying to add a site locally and call get_pv_estimate_clearsky, which will then call the buggy remove_old_cache method through the cache_response decorator. I'm getting the following error:

  File "/Users/micheal.gendy/Library/Caches/pypoetry/virtualenvs/pv-site-api-JFNXLxhc-py3.12/lib/python3.12/site-packages/fastapi/concurrency.py", line 36, in contextmanager_in_threadpool
    raise e
NameError: name 'connection' is not defined

Do I need to sort anything else out with my local? I'm just running the poetry run uvicorn pv_site_api.main:app --reload, and the dockerfile doesn't seem to be of any help so far also (failing to run the poetry install)

Side note: As a new user of the repo I think it could benefit from some more documentation around setup for prod - took a while to find where to generate credentials to get my access_code and add my own site. Happy to create another PR for some clearer docs for beginners.

hi @michael-gendy-mention-me

Thanks so much for trying this. Extra docs for how to credentials would be super. Yea, I can understand its tricky. I would set FAKE env var to 1, so the api is making fake data, not connecting to a database.

Im not sure about the connection error, can you share more of the error trace?

Thanks

peterdudfield commented 1 week ago

oh and poetry run uvicorn pv_site_api.main:app --reload should be the way to run it. Like the readme says. What docker error do you get?

michael-gendy-mention-me commented 1 week ago

@peterdudfield Ah the export FAKE 1 helped thanks - tests are now passing. The docker error was a red herring.

Still feel like I'm missing something obvious - would've thought the below would let me collect any 'fake' site uuids so I can call the/sites/{site_uuid}/clearsky_estimate endpoint and debug. But just getting 401s:

url = "http://127.0.0.1:8000/sites/"
r = requests.get(url=url, headers={"Authorization": "Bearer " + access_token})

`'{"detail":"Fail to fetch data from the url, err: \\"<urlopen error [Errno 8] nodename nor servname provided, or not known>\\""}'`

Any ideas? Do I need dev credentials or similar?

sentry-io[bot] commented 1 week ago

Sentry Issue: UK-API-2Y

peterdudfield commented 1 week ago

@peterdudfield Ah the export FAKE 1 helped thanks - tests are now passing. The docker error was a red herring.

Still feel like I'm missing something obvious - would've thought the below would let me collect any 'fake' site uuids so I can call the/sites/{site_uuid}/clearsky_estimate endpoint and debug. But just getting 401s:

url = "http://127.0.0.1:8000/sites/"
r = requests.get(url=url, headers={"Authorization": "Bearer " + access_token})

`'{"detail":"Fail to fetch data from the url, err: \\"<urlopen error [Errno 8] nodename nor servname provided, or not known>\\""}'`

Any ideas? Do I need dev credentials or similar?

Thanks again for all your work on this