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(rpi-connect): use with_state of the latest python-redux version instead of view to avoid memoization of actions - closes #248 #256

Merged
merged 1 commit into from
Mar 8, 2025

Conversation

sassanh
Copy link
Collaborator

@sassanh sassanh commented Mar 7, 2025

Some actions need data from store, for example start_service of rpi connect, needs state.lightdm.is_active so that it can decide whether it should show the "LightDM is not running" notification or not.

To provide functions, information from the store, we used to use store.view decorator but as a bad side effect, it activates memoization for the function, so calling it for the second time, won't actually run it and will just return its previous return value.

That's why "start" button of the rpi connect worked only for the first time.

In this pull request we are using the new store.with_state decorator wherever memoization is not desired in place of store.view. store.with_state is available in the latest 0.20.0 release of python-redux.

Other than that, now rpi-connect's status command, when returning the number of active sessions, uses the plural noun "sessions" for when there are multiple active sessions and singluar noun "session" when there is only one, like "1 session", "2 sessions", etc. So I made the "s" in the regular expression parsing its output optional.

… instead of `view` to avoid memoization of actions - closes #248
@sassanh sassanh requested a review from mehrdadfeller March 7, 2025 20:51
@sassanh sassanh changed the title fix(rpi-connect): use with_state of the latest python-redux version instead of view to avoid memoization of actions fix(rpi-connect): use with_state of the latest python-redux version instead of view to avoid memoization of actions - closes #249 Mar 7, 2025
Copy link
Collaborator

@mehrdadfeller mehrdadfeller left a comment

Choose a reason for hiding this comment

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

It looks good to me. It seems like the associated action has failed though.

Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ubo_app/services/050-rpi-connect/commands.py 33.33% 2 Missing ⚠️
ubo_app/services/050-rpi-connect/sign_in_page.py 0.00% 1 Missing ⚠️
ubo_app/services/050-vscode/login_page.py 0.00% 1 Missing ⚠️
Flag Coverage Δ
ubuntu-latest 56.51% <63.63%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ubo_app/display.py 78.43% <100.00%> (ø)
ubo_app/services/010-voice/setup.py 55.46% <100.00%> (ø)
ubo_app/services/080-docker/docker_container.py 21.29% <100.00%> (ø)
ubo_app/services/080-docker/docker_image.py 38.23% <100.00%> (ø)
ubo_app/services/080-docker/menus.py 38.29% <100.00%> (ø)
ubo_app/services/090-web-ui/setup.py 34.55% <100.00%> (ø)
ubo_app/services/050-rpi-connect/sign_in_page.py 51.06% <0.00%> (-1.12%) ⬇️
ubo_app/services/050-vscode/login_page.py 52.00% <0.00%> (-1.07%) ⬇️
ubo_app/services/050-rpi-connect/commands.py 35.22% <33.33%> (-0.41%) ⬇️
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sassanh sassanh changed the title fix(rpi-connect): use with_state of the latest python-redux version instead of view to avoid memoization of actions - closes #249 fix(rpi-connect): use with_state of the latest python-redux version instead of view to avoid memoization of actions - closes #248 Mar 7, 2025
@sassanh sassanh merged commit 8744e1a into main Mar 8, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants