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

[supervisor] Ensure we fetch IDEOptions/desktopIDEConfig on re-start #8226

Closed
geropl opened this issue Feb 15, 2022 · 9 comments · Fixed by #9004
Closed

[supervisor] Ensure we fetch IDEOptions/desktopIDEConfig on re-start #8226

geropl opened this issue Feb 15, 2022 · 9 comments · Fixed by #9004

Comments

@geropl
Copy link
Member

geropl commented Feb 15, 2022

So far we set dekstopIDEConfig and IDEOptions onload. With the recent change away from auto-reload behavior this does no longer show "Open in < desktopIDE >" in cases where:

  • you open a workspace in tab 1
  • stop it in tab 2 (dashboard, for example)
  • re-start it in tab 2 (dashboard, for example)

We should see
image
but instead see this:
image

The dashboard change is quite straight forward. In supervisor-frontend we'd need to re-fetch the desktopIDEConfig on re-start as well.

/cc @akosyakov

@geropl
Copy link
Member Author

geropl commented Feb 16, 2022

This might also be the cause an issue where we do not reveal the IDE of a running workspace after the tab lost focus on laptop suspension (issue):

  • we now do not auto-start if runsInFrame
  • but we have a special case for revealing for the first time a workspace becomes RUNNING
  • this breaks for re-started workspaces or laptop suspenion

I think we need to change the supervisor index to always:

  • reveal (RUNNING x browser)
  • show recent IDE options (RUNNING x desktopIDE)
    not just the "first time we see RUNNING after onload".

Internal discussion: slack

@loujaybee loujaybee moved this to Scheduled in 🚀 IDE Team Feb 21, 2022
@loujaybee
Copy link
Member

Thanks @geropl — given that this looks to be a smallish fix, let's pull into scheduled. 🙏

@akosyakov
Copy link
Member

@geropl I thought we fixed it already 🤔 ?

@geropl
Copy link
Member Author

geropl commented Feb 21, 2022

@akosyakov We only fixed the retrieval of IDEOptions. For supervisor frontend there are 2 things left (cmp esp. the comment abo ve):

  • In supervisor-frontend we'd need to re-fetch the desktopIDEConfig on re-start as well
  • I think we need to change the supervisor index to always:

    reveal (RUNNING x browser)
    show recent IDE options (RUNNING x desktopIDE)
    not just the "first time we see RUNNING after onload".

@geropl
Copy link
Member Author

geropl commented Mar 11, 2022

@loujaybee just popped up again yesterday on slack.

@akosyakov
Copy link
Member

@mustard-mh Can you have a look please? 🙏

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 28, 2022

We only update ideStatus in supervisor/frontend when ideReady, now the most direct solution is to listen to some other phase and re-listen ideReady. But this is not smart, because we don't know what phases need to wait for ideReady again, and if those phases may be missed due to other situations.

So I think the best ways to do this are

  1. Pass IDE configuration of current instance when the instance is updated. (but it lose some args like link etc, this can only generate by supervisor
  2. Or use event for ideReady to make ready multiple times?
  3. Refresh(or re-ideReady) the whole web page when instance-id changed? 👍

Way 1\2 above will change a lot. What do you think?

cc @geropl @akosyakov

@mustard-mh
Copy link
Contributor

I updated comment above, in case you missed it

@mustard-mh
Copy link
Contributor

mustard-mh commented Mar 28, 2022

So far we set dekstopIDEConfig and IDEOptions onload.

Thanks for guide here👍, I really don't know supervisor/frontend before

@akosyakov akosyakov removed the priority: highest (user impact) Directly user impacting label Mar 29, 2022
@mustard-mh mustard-mh moved this from Scheduled to Awaiting Deployment in 🚀 IDE Team Apr 7, 2022
@mustard-mh mustard-mh moved this from Awaiting Deployment to Done in 🚀 IDE Team Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
5 participants