Fix issue with late published prices

[viiru-]

• Limit backoff and maximum time to retry for AioPrices.fetch()
• Add constant to hold invalid values and use instead of repeating
• Move tomorrow_valid functionality to NordpoolData
• Only send EVENT_NEW_PRICE signal if new data is available
• Retry polling for new data hourly if needed

Fixes #326

[Hellowlol]

Hi,

Thanks for the PR. Have you tested the changes? I suspect this will generate a lot of retries and we attempt to validate every value, even the ones we don’t need or want.

[viiru-]

Thanks for the PR. Have you tested the changes? I suspect this will generate a lot of retries and we attempt to validate every value, even the ones we don’t need or want.

I have tested it by running this at different times of the day (both before and after the prices are published):

dt = datetime.now() + timedelta(days=1)
async with aiohttp.ClientSession() as session:
    spot = AioPrices('EUR', session)
    data = await spot.hourly(end_date=dt)

I haven't really looked at the number of retries produced. Probably in the beginning of the exponential backoff it retries needlessly often but that doesn't appear to be configurable in the retry/backoff implementation being used.

[Hellowlol]

Ok, please check. I assume is will retry until it succeeds, however it should not do that if we know it going to fail. Example: user restart or reloads integration at 0700 the integration will have issues getting the integration to work as it fails for tomorrows request because the prices and None because they are not available. The integration fails to be setup as it takes on longer than 10 minutes.

[viiru-]

Ok, please check. I assume is will retry until it succeeds, however it should not do that if we know it going to fail. Example: user restart or reloads integration at 0700 the integration will have issues getting the integration to work as it fails for tomorrows request because the prices and None because they are not available. The integration fails to be setup as it takes on longer than 10 minutes.

Right. I understand the problem. update_tomorrow() is called by both _someday (unnecessarily if only today is requested) which must be allowed to fail and also by new_data_cb() which must not. Adding and passing along a parameter would make things quite a bit more ugly.

Perhaps the right choice is instead to move the tomorrow_valid check to common code and run that before sending EVENT_NEW_PRICE (as this issue is caused by sending that without having valid data available)? However needing to make new_data_cb() retry itself is pretty nasty.

While reading this code I keep having the feeling of painting oneself into a corner and then applying endless workarounds to desperately try to paint back out...

[Hellowlol]

Yeah, it quickly get more complicated what you could do it to move the check after we check I’d the start and end is the same than add additional check that checks the time, it’s it less than renewal time and the call is for tomorrow ignore and raise the error is it’s today or tomorrow after the prices should be available.

You can see a example in #127

[Hellowlol]

I can do a proper review after my vacation ends. I have only checked on mobile.

[viiru-]

I'm currently thinking of the following:

1. remove new_data_cb()
2. add tomorrow_valid to NordPoolData
3. run the update from new_hr() if not tomorrow_valid and time after 13:RANDOM_MINUTE
4. after the update check tomorrow_valid again and signal if valid data was received

With those changes the component would end up retrying every hour until valid data appears. A simpler alternative would be to just poll every hour or so and just ignore the time when the auction usually finishes.

In any case the important thing is to not advertise what the aptly named add_junk() function created as valid data.

[viiru-]

@yozik04 I believe this is now "ready" in that it works and the tdo stuff has been cleaned up. Can you review again?

[yozik04]

Looks good to me.

[Hellowlol]

Can you move all the validation to aio_price? I would prefer to do it there, this allows for easier testing.

[viiru-]

That is indeed what I initially tried to do, before you explained to me why what I was doing isn't possible.

Since AioPrices doesn't keep a copy of the data (it only retrieves it, the result is stored by NordpoolData) the approach I ended up with doesn't work there.

A combination of the two variants I have so far attempted is probably possible by having AioPrices raise an exception on invalid data and handling that exception in NordpoolData (instead of having AioPrices retry like I had previously).

[viiru-]

I have now tested and pushed an implementation of that idea.

[Hellowlol]

Looks good, I havnt had time to test it yet. Ill create a beta branch so people can test it