search

TritonDataCenter / sdc-cloudapi

Triton Data Center Public HTTP API
Mozilla Public License 2.0
10 stars 24 forks source link

joyent/node-triton#128 triton ls state=provisioning` doesn't filter on state #37

Open joyent-automation opened 5 years ago

joyent-automation commented 5 years ago

joyent/node-triton#128 triton ls state=provisioning` doesn't filter on state

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

CR discussion

@marsell commented at 2017-03-03T08:50:05

Patch Set 1: Code-Review+1 Integration-Approval+1

Normally I'd ask for a test for this, but it probably would be tricky to do reliably.

@YangYong3 commented at 2017-03-15T00:16:21

Uploaded patch set 2: Patch Set 1 was rebased.

@marsell commented at 2017-04-04T23:41:19

Patch Set 2:

Yang Yong, are you planning to integrate this? It's been a while, so I'm wondering if this is in limbo.

@trentm commented at 2017-04-05T00:05:13

Patch Set 2:

Marsell,

Have you tested this? I wasn't actually sure that querying for state=provisioning to VMAPI would have results.

@marsell commented at 2017-04-05T11:06:40

Patch Set 2: -Integration-Approval

Have you tested this? I wasn't actually sure that querying for state=provisioning to VMAPI would have results.

Turns out you're right. It looked like it should work with vmapi, and that the docs were wrong, but nope: VALID_VM_STATES in common/validation.js doesn't allow it.

Is there a good reason we shouldn't add it to vmapi -- other than dealing with the ripple to other callers? Right now cloudapi gives an odd result when attempting to filter by 'provisioning': it returns all running VMs, not just VMs provisioning.

I think it'd be useful to allow filtering by provisioning, but the alternative is we disallow filtering by 'provisioning' in cloudapi; at least the results will be consistent with what was requested.

Unrelatedly, I was quite wrong about this being tricky to add tests for: it's not. If we decide to add support to vmapi for filtering by 'provisioning', I'm okay with driving all of this the rest of the way if need be (tests, dealing with other vmapi callers, etc).