-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat: sandbox deployment events #254
Changes from all commits
8bbde6d
7ad12f1
0ef8755
1d7c2f9
92a49e7
0063a4d
7a77b27
a45c889
5c58fe4
eefeaf0
bd0f86e
c861be3
6309b05
87530a6
f176750
1986cd0
ea110b3
516bb8a
c1f1c0f
40d20fb
cf25eac
75f6be7
cddb7c2
94625e7
0feeafe
05075f9
bdb2f65
c77acac
1e973a9
f98b49f
f43c10f
39d0d46
8c546d1
57ec633
3575c06
f941266
944f373
5153d15
3b93e66
42d79f2
692b804
b85d43e
d53ccb1
915b7b8
a60d541
fc86f13
aa35750
bda8e37
006628f
4813824
134f2a5
c1f2f00
e378e28
da5987d
5acd13a
8dd2666
406a9fa
f109014
2002f12
6498e71
fd3a87f
0098475
3a8dc90
a9ca6ca
6f7bf38
f17ca5a
cbc6e3b
810b694
4b7b051
135d263
5cb83a9
4105ee0
dbaed8c
cbd4e46
28a15b2
305c2ca
964b9be
49e3f11
49dc760
3da09fa
8ed1025
2ba6fd0
046eb3f
2c71a88
ff06a37
1356052
6c390d9
30f5572
a816b14
6c201eb
bd5d258
dcbcbc7
008406e
67f118a
afd1012
6607c76
f2ab03e
7885a8c
2f700db
f0b70c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@aws-amplify/sandbox': minor | ||
'@aws-amplify/backend-cli': minor | ||
--- | ||
|
||
Add event handlers for Sandbox |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,6 +72,8 @@ export class ProcessController { | |
if (typeof currentInteraction.payload === 'string') { | ||
if (currentInteraction.payload === CONTROL_C) { | ||
if (process.platform.startsWith('win')) { | ||
// Wait X milliseconds before sending kill in hopes of draining the node event queue | ||
await new Promise((resolve) => setTimeout(resolve, 5000)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. approved as long as we add a GH issue to the backlog to track this. Bonus points for linking to that issue in the comment here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
// turns out killing child process on Windows is a huge PITA | ||
// https://stackoverflow.com/questions/23706055/why-can-i-not-kill-my-child-process-in-nodejs-on-windows | ||
// https://github.com/sindresorhus/execa#killsignal-options | ||
|
This file was deleted.
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.
Based on this line it looks like the return type here should be
Promise<void>
?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.
node:EventEmitter
does not acceptPromise<void>
. It is fire and forget. See here: #254 (comment)