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: reconnect to /events if disconnected #4809

Merged
merged 3 commits into from Nov 17, 2023

Conversation

benoitf
Copy link
Collaborator

@benoitf benoitf commented Nov 14, 2023

What does this PR do?

When a provider is added, we establish a connection to /events REST API handler.
but if provider is not restarted and ping is still replying, we could have been disconnected from this stream.
It might happen if machine is restarting or some network issues.
In that case, re-establish the connection to the events so UI is properly refreshed. Because without events, we do not refresh the UI automatically

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

fixes #4712

How to test this PR?

hard to test but I've added a unit test

you can for example to test, drop the update of the status in podman extension so you can do 'podman machine stop' wait and then 'podman machine start' and it'll reconnect
you should see mesages in the dev console that event connection has been dropped

Signed-off-by: Florent Benoit fbenoit@redhat.com

@benoitf benoitf requested a review from a team as a code owner November 14, 2023 15:12
@benoitf benoitf requested review from dgolovin and cdrage and removed request for a team November 14, 2023 15:12
@benoitf
Copy link
Collaborator Author

benoitf commented Nov 15, 2023

rebased to include #4807

Copy link
Contributor

@dgolovin dgolovin left a comment

Choose a reason for hiding this comment

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

nit: no need to import Mock.
LGTM

packages/main/src/plugin/container-registry.spec.ts Outdated Show resolved Hide resolved
packages/main/src/plugin/container-registry.spec.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

fixes containers#4712
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf benoitf merged commit 83630c2 into containers:main Nov 17, 2023
9 checks passed
@benoitf benoitf deleted the DESKTOP-4712 branch November 17, 2023 07:37
@podman-desktop-bot podman-desktop-bot added this to the 1.6.0 milestone Nov 17, 2023
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.

Connection to /events drops sometimes
4 participants