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

Use a sentinel in LatestValueCache to denote if the cache is empty #846

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

shsms
Copy link
Contributor

@shsms shsms commented Jan 19, 2024

This is necessary because None is a valid value that could have been
sent through a channel, so the having a None value might not always
mean that the cache is empty.

Closes #820

This is necessary because `None` is a valid value that could have been
sent through a channel, so the having a `None` value might not mean
that the cache is empty.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms shsms requested a review from a team as a code owner January 19, 2024 14:46
@shsms shsms requested a review from llucax January 19, 2024 14:46
@github-actions github-actions bot added part:actor Affects an actor ot the actors utilities (decorator, etc.) part:core Affects the SDK core components (data structures, etc.) labels Jan 19, 2024
@shsms shsms added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jan 19, 2024
@shsms
Copy link
Contributor Author

shsms commented Jan 19, 2024

This is an internal fix for something that was introduced in this release cycle, so no release notes necessary.

@shsms shsms self-assigned this Jan 19, 2024
@shsms shsms added this to the v1.0.0-rc4 milestone Jan 19, 2024
Copy link
Contributor

@daniel-zullo-frequenz daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

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

I've only have a comment to check for. LGTM otherwise

The previous commit has introduced the possibility for exceptions to
be raised from the implementation of `latest_value` property.

Properties should ideally not raise, so we replace it with the `get`
method.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@github-actions github-actions bot added the part:data-pipeline Affects the data pipeline label Jan 24, 2024
@shsms shsms added this pull request to the merge queue Jan 24, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit b425905 Jan 24, 2024
14 checks passed
@shsms shsms deleted the latest-value-cache-fixes branch January 24, 2024 09:59
@llucax llucax removed this from the v1.0.0-rc4 milestone Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd:skip-release-notes It is not necessary to update release notes for this PR part:actor Affects an actor ot the actors utilities (decorator, etc.) part:core Affects the SDK core components (data structures, etc.) part:data-pipeline Affects the data pipeline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LatestValueCache can't use None as a valid value
3 participants