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

Immediate device updates & limit attributes to be updated #34

Merged

Conversation

tsagadar
Copy link
Collaborator

I was investigating the issue, that sometimes an assigned firmware got reset. It showed to be a combination of active UI and device polling. The Tortoise ORM logs were not that helpful as device updates always updated all attributes.

Refactored the code that it explicitly lists what should be updated - and does so immediately after modifying the device state. This either fixed the above issue or at least will help to more easily track it in the future.

tsagadar added 2 commits July 28, 2024 14:27
UpdateManager update_xyz methods will immediately persist state to database.
Prevents device in-memory state to diverge from database state. Also validated
that only UpdateManager updates device state.

Explicitly listing attributes to be updated allows to more easily match
Tortoise ORM debug logs to the updates that took place. Could help to pinpoint
the issue with the assigned firmware being overwritten - or even fix it.
Copy link
Contributor

@b-rowan b-rowan left a comment

Choose a reason for hiding this comment

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

Couple small things to confirm.

goosebit/updater/manager.py Show resolved Hide resolved
goosebit/updater/routes.py Outdated Show resolved Hide resolved
tsagadar added 6 commits July 28, 2024 21:54
Detail message "jwk.py:58: DeprecationWarning: Please use a Key object
instead of bytes or string."
This extends the test coverage and no longer relies on internals (db state)
to validate state changes.
Copy link
Contributor

@b-rowan b-rowan left a comment

Choose a reason for hiding this comment

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

Looks good to merge if you're happy with everything?

@tsagadar tsagadar merged commit 47103f7 into UpstreamDataInc:master Jul 29, 2024
@tsagadar tsagadar deleted the mm/issue-with-assigned-updates branch July 29, 2024 04:41
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