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

OS-7749 VM.js methods using vminfod event stream should timeout on vminfod events, not on the VM actions #869

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joyent-automation
Copy link
Contributor

OS-7749 VM.js methods using vminfod event stream should timeout on vminfod events, not on the VM actions

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/6190/.
The raw archive of this CR is here.
See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@trentm commented at 2019-05-02T23:19:57

Patch Set 1:

New commits:
commit 32a47bb
stage 1: VmWatcher working and VM.deleteZone using it; tests pass

commit dad8aa961cb82de645fab7183e8117fca4ec883e  
doc updates; add function names to deleteZone
@trentm commented at 2019-05-02T23:22:28

Patch Set 1: Code-Review-1

-1 because this is still a draft for now

@bahamas10 commented at 2019-05-03T16:38:06

Patch Set 1:

(4 comments)

+1 to this new interface! I really like it, and like the fact that it can be used in a pipeline and not a parallel - it makes it very clear what runs first, and in what order things are spun up.

Patch Set 1 code comments
src/vm/man/vmadm.1m.md#97 @bahamas10

I'm not actually sure what the [<...>] syntax means vs. the [...] syntax.

src/vm/node_modules/VM.js#18595 @bahamas10

assert.uuid(uuid, 'uuid');

above this line

src/vm/node_modules/vminfod/client.js#1156 @bahamas10

What happens in the event that the events were already seen? i.e.: if they were seen as part of the zoneadm call above? Looking through the code, it appears that vmwatcher.wait will push to self._waiters. However, I also see that this array is changed to null once the match is complete.

Could this call to wait potentially blow up in the event that the match is already met, as a value will be pushed to a null object?

src/vm/node_modules/vminfod/client.js#1400 @bahamas10

oh, very useful!

@trentm commented at 2019-05-03T17:04:40

Patch Set 2:

New commits:
commit c387a85
stage 1: VmWatcher working and VM.deleteZone using it; tests pass

commit 613fc65dcdf6a76fe605d33f7d82cb7084e8ff66  
doc updates; add function names to deleteZone

@trentm trentm closed this Sep 30, 2020
@bahamat bahamat reopened this May 3, 2021
@trentm trentm removed their assignment Feb 25, 2023
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.

3 participants