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

When doing Context > Use in explorer view, takes 20s for "Connected" icon to move #1965

Closed
bwateratmsft opened this issue May 11, 2020 · 11 comments · Fixed by #1969
Closed
Assignees
Milestone

Comments

@bwateratmsft
Copy link
Collaborator

  1. Create a Docker context
  2. In Explorer view, right click it, "Use"

Expected:
"Connected" icon should move quickly

Actual:
"Connected" icon takes 20s to move over, long after the containers/images/etc. views are already refreshed with the new context

image

@bwateratmsft
Copy link
Collaborator Author

We should make our context->use command immediately change the icon.

@ravipal
Copy link
Contributor

ravipal commented May 11, 2020

After context switch, we do refresh the context tree, so user don't have to wait for the refresh interval. I can't repro this issue using my contexts. @bwateratmsft can you repro this consistently? Does this happen only when you switch to a specific context. Also how long does the command 'docker context ls' takes when you run from CLI

@karolz-ms
Copy link
Contributor

I have seen this. I suspect the call to refresh context tree is queued behind calls to refresh other panes. This is esp. possible if default 2-sec refresh period was retained when switching to remote context.

Instead of expediting the panel refresh we should just change the view model immediately to indicate the new active context.

@ravipal
Copy link
Contributor

ravipal commented May 12, 2020

Added some traces to debug the issue and it seems like sometimes the command docker context use is not instantaneous. In my machine most of the time it completes within a second and sometime it takes 8 seconds.
I don't think updating the view model would solve the delay, if the context switch command is taking long time. It could improve a bit and don't have to wait for the actual refresh.
The other option is switch the icon right away even before the docker context use completes.

@bwateratmsft
Copy link
Collaborator Author

Why would updating the view model not work?

@karolz-ms
Copy link
Contributor

The context switch may indeed take a long time to complete. Consider having a progress indicator while docker context use command is running. Once it is complete, we should update the view model to indicate the new context in use.

@bwateratmsft
Copy link
Collaborator Author

I know for at least some parts of the whole operation we have a progress indicator above the Contexts panel, but it seemed to be a little flickery. That said, above the contexts panel isn't very visible, so I'd like to have a progress notification for as long as it takes to get the context switched. When the switching operation finishes, we can update the view model to show the new "Current" node, and the containers/images/etc. views will automatically refresh within 2 seconds.

@dbreshears dbreshears added this to the 1.3.0 milestone May 12, 2020
@BigMorty BigMorty added the P2 label May 12, 2020
@ravipal
Copy link
Contributor

ravipal commented May 12, 2020

I sent a private bit for you to try. The new behavior is

  1. When context switch is in progress, no new auto refresh will be triggered on any panels.
  2. A progress icon will be shown on the context that is being switched to and the description will say 'Using...'

@bwateratmsft
Copy link
Collaborator Author

@ravipal @BigMorty In order to get this into 1.2.1 we will need to dual-checkin. I'll create a rel/1.2.1 branch.

@BigMorty
Copy link
Member

We should definitely get this into 1.2.1.

@bwateratmsft
Copy link
Collaborator Author

Fix released in 1.2.1.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants