Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Nov 25, 2024

When awaiting the resamplers to finish resampling, we need to make a copy of the resamplers dict, because new resamplers could be added or removed while we are awaiting, which would cause the results to be out of sync.

Here is a traceback of an instance of this bug:

2024-11-25 12:45:26,052,52 WARNING  [_broadcast.py:416] Broadcast receiver [Broadcast:GrpcStreamBroadcaster-raw-component-data-2006:_Receiver] is full. Oldest message was dropped.
2024-11-25 12:45:26,052,52 ERROR    [_resampling.py:195] The resample() function got an unexpected error, restarting...
Traceback (most recent call last):
  File "/home/marenz/frequenz/sdk/src/frequenz/sdk/microgrid/_resampling.py", line 179, in _log_resampling_task_error
    resampling_task.result()
  File "/home/marenz/frequenz/sdk/src/frequenz/sdk/timeseries/_resampling.py", line 509, in resample
    {
  File "/home/marenz/frequenz/sdk/src/frequenz/sdk/timeseries/_resampling.py", line 514, in <dictcomp>
    if isinstance(results[i], (Exception, asyncio.CancelledError))
                  ~~~~~~~^^^
IndexError: list index out of range

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner November 25, 2024 15:47
@llucax llucax requested review from ela-kotulska-frequenz and removed request for a team November 25, 2024 15:47
@github-actions github-actions bot added part:docs Affects the documentation part:data-pipeline Affects the data pipeline labels Nov 25, 2024
@llucax llucax self-assigned this Nov 25, 2024
@llucax llucax changed the title IndexError: list index out of range Fix IndexError: list index out of range in resampler Nov 25, 2024
@llucax llucax modified the milestones: v1.0.0-rc1301, v1.0.0-rc1302 Nov 25, 2024
@llucax llucax added the type:bug Something isn't working label Nov 25, 2024
@llucax llucax enabled auto-merge November 26, 2024 10:21
When awaiting the resamplers to finish resampling, we need to make a
copy of the resamplers dict, because new resamplers could be added or
removed while we are awaiting, which would cause the results to be
out of sync.

Signed-off-by: Leandro Lucarella <[email protected]>
Either `mypy` was improved in general or using `zip()` helped it to
figure out the type narrowing, but it seems it can now narrow the type
of `exceptions` without the need of a cast.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax force-pushed the resampler-out-of-bounds branch from ba57eb1 to 86430a7 Compare November 27, 2024 11:09
@llucax
Copy link
Contributor Author

llucax commented Nov 27, 2024

Updated to:

  • Copy only the keys
  • Use zip()
  • Remove the unnecessary cast

Copy link
Contributor

@ela-kotulska-frequenz ela-kotulska-frequenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@llucax llucax added this pull request to the merge queue Nov 27, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit 9cec69a Nov 27, 2024
18 checks passed
@llucax llucax deleted the resampler-out-of-bounds branch November 27, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:data-pipeline Affects the data pipeline part:docs Affects the documentation type:bug Something isn't working

Projects

Development

Successfully merging this pull request may close these issues.

2 participants