- Notifications
You must be signed in to change notification settings - Fork1.1k
fix(UI): workspace restart button stops build before starting a new one#7301
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
* Refactor primary buttons* refactor(site): Always show the main actions* Remove tests that are testes on Storybook* Fix tests* Fix keys* added restart btn---------Co-authored-by: BrunoQuaresma <bruno_nonato_quaresma@hotmail.com>
Uh oh!
There was an error while loading.Please reload this page.
| } | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Originally, I wanted to use an implementation closer to the following, which react query'sdocs suggest is possible:
const requestStopWorkspace = useMutation({ mutationFn: stopWorkspace }) const requestStartWorkspace = useMutation({ mutationFn: startWorkspace }) const restartWorkspace = async () => { try { await requestStopWorkspace.mutateAsync(workspace.id) await waitForStop(requestStopWorkspace.data) await requestStartWorkspace.mutateAsync({ workspaceId, templateVersionId, }) } catch (error) { ... } }butmutateAsync does not seem to be working and the queries are not treated asynchronously.@BrunoQuaresma is this something you've run into before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I put a comment above, I think you don't need to have each API call into a "query". You can have a single function to restart the workspace and wrap this function into a query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hmmm that's a good idea. Let me try it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@BrunoQuaresma Woo that's a lot better! Thanks!
ammario commentedApr 26, 2023
Thank you for fixing this so quickly :) |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
bpmct left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Works as expected with AWS template (stops workspace then starts)
Resolves#6241 and replaces the reverted#7268.
I've added a test to ensure the restart button hook is called appropriately but React Query makes it difficult to test side effects so I'm effectively mocking the
stopWorkspaceapi call and not thestartWorkspaceapi call. I will take a second look at this during a quieter day, but for the time being, smoke testing would be appreciated!