Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IndexError: list index out of range in resampler #1117

Merged
merged 4 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# Frequenz Python SDK Release Notes

## Summary

<!-- Here goes a general summary of what this release is about -->

## Upgrading

<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->

## New Features

<!-- Here goes the main new features and examples or instructions on how to use them -->

## Bug Fixes

* Fix bug with `LoggingConfigUpdater` not updating root logger level.
* The `frequenz-quantities` dependency requirement was widened to allow any v1.x version (it was pinned to `1.0.0rc3` before).
- Fix a bug in the resampler that could end up with an *IndexError: list index out of range* exception when a new resampler was added while awaiting the existing resampler to finish resampling.
26 changes: 12 additions & 14 deletions src/frequenz/sdk/timeseries/_resampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from collections.abc import AsyncIterator, Callable, Coroutine, Sequence
from dataclasses import dataclass
from datetime import datetime, timedelta, timezone
from typing import cast

from frequenz.channels.timer import Timer, TriggerAllMissed, _to_microseconds
from frequenz.quantities import Quantity
Expand Down Expand Up @@ -495,25 +494,24 @@ async def resample(self, *, one_shot: bool = False) -> None:
self._config.resampling_period,
)

# We need to make a copy here because we need to match the results to the
# current resamplers, and since we await here, new resamplers could be added
# or removed from the dict while we awaiting the resampling, which would
# cause the results to be out of sync.
resampler_sources = list(self._resamplers)
results = await asyncio.gather(
*[r.resample(self._window_end) for r in self._resamplers.values()],
return_exceptions=True,
)

self._window_end += self._config.resampling_period
# We need the cast because mypy is not able to infer that this can only
# contain Exception | CancelledError because of the condition in the list
# comprehension below.
exceptions = cast(
dict[Source, Exception | asyncio.CancelledError],
{
source: results[i]
for i, source in enumerate(self._resamplers)
# CancelledError inherits from BaseException, but we don't want
# to catch *all* BaseExceptions here.
if isinstance(results[i], (Exception, asyncio.CancelledError))
},
)
exceptions = {
source: result
for source, result in zip(resampler_sources, results)
# CancelledError inherits from BaseException, but we don't want
# to catch *all* BaseExceptions here.
if isinstance(result, (Exception, asyncio.CancelledError))
}
if exceptions:
raise ResamplingError(exceptions)
if one_shot:
Expand Down
Loading