Skip to content

fix: Allowed multiple update callbacks on an AttrR#128

Merged
evvaaaa merged 1 commit intomainfrom
127-allow-multiple-update-callbacks
Mar 12, 2025
Merged

fix: Allowed multiple update callbacks on an AttrR#128
evvaaaa merged 1 commit intomainfrom
127-allow-multiple-update-callbacks

Conversation

@evvaaaa
Copy link
Contributor

@evvaaaa evvaaaa commented Mar 10, 2025

  • Transports will add an update callback, so if we don't allow for multiple then running them simultaneously won't work.
  • The controller level can add their own now too.

@evvaaaa evvaaaa requested review from GDYendell and jsouter March 10, 2025 11:02
@codecov
Copy link

codecov bot commented Mar 10, 2025

Codecov Report

Attention: Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.56%. Comparing base (759fa13) to head (a4c4499).

Files with missing lines Patch % Lines
src/fastcs/backend.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   90.52%   90.56%   +0.03%     
==========================================
  Files          41       41              
  Lines        1920     1928       +8     
==========================================
+ Hits         1738     1746       +8     
  Misses        182      182              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jsouter
Copy link
Contributor

jsouter commented Mar 10, 2025

Looks good, I'll check if with fastcs-eiger

@evvaaaa evvaaaa self-assigned this Mar 10, 2025
@evvaaaa evvaaaa linked an issue Mar 10, 2025 that may be closed by this pull request
@evvaaaa
Copy link
Contributor Author

evvaaaa commented Mar 10, 2025

Looks good, I'll check if with fastcs-eiger

I imagine it wouldn't work because of other changes. We've just found a bug in the last PR where now the controller that comes through in handlers is always the top level one.

@evvaaaa evvaaaa force-pushed the 127-allow-multiple-update-callbacks branch 2 times, most recently from 8b3e2ea to a8ccffd Compare March 10, 2025 14:41
@evvaaaa evvaaaa requested a review from GDYendell March 11, 2025 10:23
* Transports will add an update callback, so if we don't allow for
multiple then running them simultaneously won't work.
* The controller level can add their own now too.
@evvaaaa evvaaaa force-pushed the 127-allow-multiple-update-callbacks branch from a8ccffd to a4c4499 Compare March 11, 2025 10:24
@evvaaaa evvaaaa merged commit 920a8b2 into main Mar 12, 2025
18 checks passed
@evvaaaa evvaaaa deleted the 127-allow-multiple-update-callbacks branch March 12, 2025 08:12
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.

Allow for multiple _update_callback on attributes

3 participants

Comments