Publish workflow invocation workspace status event sooner #6529
Conversation
| // Note: COMMIT_SHA may not actually reflect the current state of the | ||
| // repo since we merge the target branch before running the workflow; | ||
| // we set this for the purpose of reporting statuses to GitHub. | ||
| {Key: "COMMIT_SHA", Value: *commitSHA}, |
There was a problem hiding this comment.
One thing to note is that we sometimes set commitSha in sync() if it was not passed in as a flag (i.e. only a branch was passed). For that case, now the UI won't contain the commit sha
That should only happen in the ExecuteWorkflow API though, and in most cases we'd expect both to be set, so I think this is still worth it
There was a problem hiding this comment.
If this becomes a major issue for people, we could write to the DB twice, once when seeing WorkflowConfigured and once when seeing WorkspaceStatus, then move this back to after the git repo is fetched
There was a problem hiding this comment.
hmm actually I think this would affect any users who are currently using the ExecuteWorkflow API. For those people, we would wind up setting COMMIT_SHA to an empty string which would break GH status publishing.
@tylerwilliams wdyt about allowing workflows to do 1 extra invocation DB write - once when getting WorkflowConfigured, and once after checking out the git repo (which is when we can reliably compute WorkspaceStatus)? The problem it'd solve is that today, we don't always know the commit_sha until checking out the repo, but if the git fetch is slow then the user is stuck in this "Invocation in progress..." page until it's done, rather than being able to see incremental progress
|
Will close in favor of saving the invocation row when we get WorkflowConfigured. |
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