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

Small debugging enhancements #1119

Merged
merged 6 commits into from
Nov 28, 2024
Merged

Small debugging enhancements #1119

merged 6 commits into from
Nov 28, 2024

Conversation

Marenz
Copy link
Contributor

@Marenz Marenz commented Nov 27, 2024

  • Fix wrong term in battery formula documentation
  • Give more receivers/tasks a name for easier debugging
  • Add a nice str representation for Sample
  • Actually log the error that caused a source to be removed
  • Add more details to "no relevant sample found" warning log
  • Add change log

@Marenz Marenz requested a review from a team as a code owner November 27, 2024 10:00
@Marenz Marenz requested review from daniel-zullo-frequenz and removed request for a team November 27, 2024 10:00
@github-actions github-actions bot added part:docs Affects the documentation part:data-pipeline Affects the data pipeline part:microgrid Affects the interactions with the microgrid labels Nov 27, 2024
@Marenz Marenz force-pushed the little branch 2 times, most recently from 20b0e3d to 94d72a8 Compare November 27, 2024 10:04
Comment on lines 457 to 459
request_receiver=channel.new_receiver(
limit=_REQUEST_RECV_BUFFER_SIZE, name=channel.name + " Receiver"
),
Copy link
Contributor

@llucax llucax Nov 27, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using .name directly, yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

src/frequenz/sdk/timeseries/_base_types.py Outdated Show resolved Hide resolved
src/frequenz/sdk/microgrid/_resampling.py Outdated Show resolved Hide resolved
src/frequenz/sdk/timeseries/_resampling.py Show resolved Hide resolved
src/frequenz/sdk/timeseries/_resampling.py Outdated Show resolved Hide resolved
Easier to read & compare, especially when debugging.

Signed-off-by: Mathias L. Baumann <[email protected]>
@Marenz Marenz force-pushed the little branch 2 times, most recently from 5a4e391 to 80197eb Compare November 27, 2024 14:35
@Marenz Marenz enabled auto-merge November 27, 2024 16:40
llucax
llucax previously approved these changes Nov 28, 2024
Copy link
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

LGTM except for the DEBUG -> WARNING. Feel free to force-merge after it is fixed.

minimum_relevant_timestamp: Minimum timestamp that was requested
timestamp: Timestamp that was requested
"""
if not _logger.isEnabledFor(logging.DEBUG):
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be WARNING!

Suggested change
if not _logger.isEnabledFor(logging.DEBUG):
if not _logger.isEnabledFor(logging.WARNING):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shit. So much for my FCR deployment.

@Marenz Marenz added this pull request to the merge queue Nov 28, 2024
@llucax llucax removed this pull request from the merge queue due to a manual request Nov 28, 2024
@llucax llucax dismissed their stale review November 28, 2024 07:46

Accidental approval

@llucax llucax enabled auto-merge November 28, 2024 07:46
Signed-off-by: Mathias L. Baumann <[email protected]>
@Marenz Marenz disabled auto-merge November 28, 2024 12:31
@Marenz Marenz merged commit 5d337b6 into frequenz-floss:v1.x.x Nov 28, 2024
18 checks passed
@Marenz Marenz deleted the little branch November 28, 2024 15:24
@llucax llucax added this to the v1.0.0-rc1302 milestone Nov 28, 2024
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 part:microgrid Affects the interactions with the microgrid
Projects
Development

Successfully merging this pull request may close these issues.

2 participants