Skip to content

Add function:sendHeartBeat scope #11021

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 13, 2022
Merged

Add function:sendHeartBeat scope #11021

merged 1 commit into from
Jul 13, 2022

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Jun 30, 2022 •
edited by jeanp413
Loading

Description

Add function:sendHeartBeat scope for vscode clients

Release Notes

NONE

Werft options:

  • /werft with-preview

@akosyakov
Copy link
Member Author

akosyakov commented Jun 30, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jp-heartbeat-test.3
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-jp-heartbeat-test.2 because the annotations in the pull request description changed
(with .werft/ from main)

@akosyakov akosyakov mentioned this pull request Jun 30, 2022
Merged
@jeanp413
Copy link
Member

jeanp413 commented Jul 2, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jp-heartbeat-test.4
(with .werft/ from main)

@akosyakov
Copy link
Member Author

akosyakov commented Jul 6, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Don't forget that it should be deployed before we release new version of VS Code Desktop extension 🙏 Maybe we should open it for review already?

@jeanp413
Copy link
Member

jeanp413 commented Jul 6, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-jp-heartbeat-test.5
(with .werft/ from main)

@jeanp413 jeanp413 marked this pull request as ready for review July 6, 2022 15:57
@jeanp413 jeanp413 requested a review from a team July 6, 2022 15:57
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 6, 2022
@jeanp413
Copy link
Member

jeanp413 commented Jul 6, 2022

/hold

@geropl
Copy link
Member

geropl commented Jul 7, 2022

@jeanp413 Could you elaborate on the motivation for this change?
It sounds reasonable, but why did it work beforehand? I thought supervisor was responsible for sending hearbeats - why extending it to clients? 🤔

@geropl geropl self-assigned this Jul 7, 2022
@jeanp413
Copy link
Member

jeanp413 commented Jul 7, 2022

@geropl previous way still works, but we are trying to improve it in the cases where there's a disconnection, if user looses ssh connection in vscode desktop then as long as it has some activity in the vscode local window heartbeat should be send so workspace doesn't stop due to the 5 min timeout

@mustard-mh
Copy link
Contributor

Rebased to main to make preview env available

@geropl
Copy link
Member

geropl commented Jul 13, 2022

@jeanp413 @mustard-mh I'm unsure in what state this PR is in. Can we either merge it or move it back to draft state, please? 🙏

@jeanp413 jeanp413 force-pushed the jp/heartbeat-test branch from 13d5e92 to bfb3f24 Compare July 13, 2022 14:33
@jeanp413
Copy link
Member

Sorry, I'll merge, was using the prev env to test it.

@jeanp413
Copy link
Member

/unhold

@roboquat roboquat merged commit 0107eaf into main Jul 13, 2022
@roboquat roboquat deleted the jp/heartbeat-test branch July 13, 2022 14:42
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geropl geropl geropl approved these changes

Assignees

@geropl geropl

Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@akosyakov @jeanp413 @geropl @mustard-mh @roboquat