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

Publish workflow invocation workspace status event sooner (take 2) #6691

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

bduffany
Copy link
Member

@bduffany bduffany commented May 31, 2024

Publish the workspace status sooner, but only if commit_sha is known. Otherwise, wait until we sync the repo (to get the resolved HEAD commit SHA), but log a warning.

The warning focuses on the reproducibility benefits (making sure the build happens at an exact commit SHA), even though this PR is focused on the UX benefit of showing results in the UI sooner. I figured the reproducibility benefit might be a better selling point for getting people to set the commit_sha.


(Original context from #6529)

WorkspaceStatus is one of the events that prevents us from writing the Invocation row to the DB (due to the code here and here), meaning the UI will be stuck in "Invocation is in progress..." state while we are cloning the git repo. This PR moves the WorkspaceStatus event to be published before attempting to sync the git repo, so that we can get a nicer invocation page sooner even if the git repo sync is taking longer than usual.

Verified by adding an artificial 10s sleep to ws.setup() locally, and checking that this results in "Invocation in progress..." state being displayed for at least 10s, while after the fix, we can see the full invocation page while the sleep is happening.

Related issues: N/A

@bduffany bduffany requested a review from maggie-lou May 31, 2024 21:45
@bduffany bduffany merged commit 861d44e into master Jun 4, 2024
17 of 18 checks passed
@bduffany bduffany deleted the wf-workspace-status-2 branch June 4, 2024 18:47
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.

None yet

2 participants