Fox Cloud retry backoff, savings calc update change to 1am #3108
Conversation
springfall2008
changed the title
There was a problem hiding this comment.
Pull request overview
This pull request changes the savings calculation update timing from midnight to 1 AM and adds rate limiting protection to the Fox API integration to prevent excessive API usage.
Key changes:
- Savings totals now update at 1 AM instead of midnight to allow cloud data to stabilize
- Fox API rate limiting tracks request rates and disables retries when exceeding 60 requests/hour
- Daily counters reset at midnight to track per-day API usage
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| apps/predbat/predbat.py | Modified savings calculation to update at 1 AM (after 60 minutes past midnight) instead of at midnight, allowing cloud data to stabilize before daily totals are calculated |
| apps/predbat/fox.py | Added rate limiting logic including request tracking, hourly rate calculation with 30-minute floor, midnight reset of counters, and retry prevention when exceeding 60 requests/hour |
| apps/predbat/tests/test_fox_api.py | Added comprehensive test coverage for rate limiting scenarios, midnight resets, available variables API, and updated mock objects to support rate limiting attributes |
apps/predbat/tests/test_fox_api.py
Outdated
| if fox.last_midnight_utc is not None and fox.last_midnight_utc != current_midnight: | ||
| fox.requests_today = 0 | ||
| fox.rate_limit_errors_today = 0 | ||
| fox.start_time_today = datetime.now(timezone.utc) |
There was a problem hiding this comment.
The test on line 4877 uses datetime.now(timezone.utc) directly instead of using the mocked datetime. Since the patch on line 4869 patches fox.datetime which doesn't correctly intercept the imported datetime function, this call will use the real datetime.now() instead of the mocked time. This will cause the test assertion on line 4882 to potentially fail because start_time_today will be set to the actual current time rather than the mocked day2_time.
| fox.start_time_today = datetime.now(timezone.utc) | |
| fox.start_time_today = mock_datetime.now(timezone.utc) |
| # Increment total at midnight for next day, don't do if in read-only mode | ||
| if savings_total_last_updated and savings_total_last_updated != todays_date and scheduled and recompute and not self.set_read_only: | ||
| # Increment total at 1am once we have today's data stable (cloud data can lag) | ||
| if self.minutes_now > 60 and savings_total_last_updated and savings_total_last_updated != todays_date and scheduled and recompute and not self.set_read_only: |
There was a problem hiding this comment.
The condition self.minutes_now > 60 only checks if it's past 1:00 AM. However, this check will always be true during the rest of the day (1:05 AM, 2:00 PM, etc.), which means the savings update logic can potentially trigger multiple times per day instead of once. Consider using a more precise check such as self.minutes_now >= 60 and self.minutes_now < 65 or add an additional flag to ensure the update only happens once per day at 1 AM.
| if self.minutes_now > 60 and savings_total_last_updated and savings_total_last_updated != todays_date and scheduled and recompute and not self.set_read_only: | |
| if self.minutes_now >= 60 and self.minutes_now < 65 and savings_total_last_updated and savings_total_last_updated != todays_date and scheduled and recompute and not self.set_read_only: |
| with patch("fox.datetime") as mock_datetime: | ||
| # Mock datetime.now() to return our test time | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| with patch("fox.datetime") as mock_datetime: | |
| # Mock datetime.now() to return our test time | |
| mock_datetime.now.return_value = current_time | |
| with patch("fox.datetime.now") as mock_datetime_now: | |
| # Mock datetime.now() to return our test time | |
| mock_datetime_now.return_value = current_time |
| with patch("fox.datetime") as mock_datetime: | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| with patch("fox.datetime") as mock_datetime: | ||
| # Set start time (30 minutes minimum) | ||
| fox.start_time_today = start_time | ||
| current_time = start_time + timedelta(minutes=30) | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| with patch("fox.datetime") as mock_datetime: | |
| # Set start time (30 minutes minimum) | |
| fox.start_time_today = start_time | |
| current_time = start_time + timedelta(minutes=30) | |
| mock_datetime.now.return_value = current_time | |
| with patch("fox.datetime.now") as mock_datetime_now: | |
| # Set start time (30 minutes minimum) | |
| fox.start_time_today = start_time | |
| current_time = start_time + timedelta(minutes=30) | |
| mock_datetime_now.return_value = current_time |
| with patch("fox.datetime") as mock_datetime: | ||
| fox.start_time_today = start_time | ||
|
|
||
| # Scenario 1: 20 requests in first 5 minutes (would be 240/hour without floor!) | ||
| current_time = start_time + timedelta(minutes=5) | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| with patch("fox.datetime") as mock_datetime: | ||
| fox.start_time_today = start_time | ||
|
|
||
| # Hour 1: 20 requests | ||
| current_time = start_time + timedelta(hours=1) | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| if self.minutes_now > 60 and savings_total_last_updated and savings_total_last_updated != todays_date and scheduled and recompute and not self.set_read_only: | ||
| savings_total_predbat += self.savings_today_predbat | ||
| savings_total_pvbat += self.savings_today_pvbat | ||
| savings_total_soc = self.savings_today_predbat_soc |
There was a problem hiding this comment.
The assignment to savings_total_soc should use the compound assignment operator += to accumulate the total, not simple assignment =. This is inconsistent with the other savings totals being accumulated on lines 827-828, 830, and 832.
| savings_total_soc = self.savings_today_predbat_soc | |
| savings_total_soc += self.savings_today_predbat_soc |
| with patch("fox.datetime") as mock_datetime: | ||
| # Start with high rate (35 requests in 30 minutes = 70/hour) | ||
| fox.start_time_today = start_time | ||
| current_time = start_time + timedelta(minutes=30) | ||
| mock_datetime.now.return_value = current_time |
There was a problem hiding this comment.
The test uses patch("fox.datetime") to mock datetime, but the actual code in fox.py imports datetime at the top with from datetime import datetime, timedelta, timezone. This means datetime.now() in the code is a direct reference to the function, not an attribute of a module, so patch("fox.datetime") won't correctly intercept the call. The patch should be patch("fox.datetime.now") or you need to patch at the point where it's used as datetime.now.
| elapsed_seconds = max((current_time - start_time).total_seconds(), 1800) | ||
| hourly_rate = (fox.requests_today * 3600) / elapsed_seconds |
There was a problem hiding this comment.
This assignment to 'hourly_rate' is unnecessary as it is redefined before this value is used.
| elapsed_seconds = max((current_time - start_time).total_seconds(), 1800) | |
| hourly_rate = (fox.requests_today * 3600) / elapsed_seconds |
…l2008#3108) * Fox API retry backoff * Change savings calculation to update at 1am * Fix issue with fox test
No description provided.