Skip to content

Fetch dispatches in sync with periodic reconnect of client.stream #155

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

Open
wants to merge 1 commit into
base: v0.x.x
Choose a base branch
from

Conversation

Marenz
Copy link
Contributor

@Marenz Marenz commented May 21, 2025

Should probably use
frequenz-floss/frequenz-client-base-python#141
eventually to be less hacky?

@github-actions github-actions bot added part:docs Affects the documentation part:dispatcher Affects the high-level dispatcher interface labels May 21, 2025
@Marenz Marenz force-pushed the periodic-event-check branch from ba62a7a to a79b3e3 Compare May 26, 2025 10:19
@github-actions github-actions bot added the part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) label May 26, 2025
@Marenz Marenz changed the title Reconnect stream if there has been no event in 5 minutes Fetch dispatches in sync with periodic reconnect of client.stream May 26, 2025
@Marenz Marenz marked this pull request as ready for review May 26, 2025 10:21
@Marenz Marenz requested a review from a team as a code owner May 26, 2025 10:21
@Marenz Marenz added the status:blocked Other issues must be resolved before this can be worked on label May 26, 2025
@Marenz
Copy link
Contributor Author

Marenz commented May 26, 2025

@Marenz Marenz requested a review from llucax May 26, 2025 16:47
To ensure we didn't miss any.

Signed-off-by: Mathias L. Baumann <[email protected]>
@Marenz Marenz force-pushed the periodic-event-check branch from a79b3e3 to b5aff78 Compare May 27, 2025 16:32
Copy link
Contributor

@ela-kotulska-frequenz ela-kotulska-frequenz left a comment

Choose a reason for hiding this comment

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

LGTM, only needs rebasing :)

@Marenz
Copy link
Contributor Author

Marenz commented May 28, 2025

LGTM, only needs rebasing :)

Unfortunately, using a new base-client also means allowing a new base client version in microclient, so we will have to release both base-client and microclient

@llucax
Copy link
Contributor

llucax commented Jun 2, 2025

"microclient" 😆

await self._lifecycle_events_tx.send(Deleted(dispatch=dispatch))
match selected.message:
case StreamStartedEvent():
_logger.info("Dispatch stream restarted, refreshing dispatches")
Copy link
Contributor

Choose a reason for hiding this comment

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

"(re)started"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Realistically we never will get the start notification because we create the new_receiver() only after it was sent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:dispatcher Affects the high-level dispatcher interface part:docs Affects the documentation part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) status:blocked Other issues must be resolved before this can be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants