feat: differentiate simple/advanced API for fabrica payloads - #131
feat: differentiate simple/advanced API for fabrica payloads#131synackd wants to merge 1 commit into
Conversation
Implement API changes from OpenCHAMI/fabrica#64. Add --envelope flag to fabrica resource commands to allow the user to specify additional metadata that envelops the spec, which contains the actual data. By default (without --envelope), the payload is just the spec and additional metadata is ignored. Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
| @@ -27,38 +29,67 @@ See ochami-boot(1) for more details.`, | |||
| Example: ` # Add BMC using payload data | |||
| ochami boot bmc add -d \ | |||
There was a problem hiding this comment.
Shouldn't we have -e for the full envelope here?
| }' | ||
|
|
||
| # Add multiple BMCs using payload data | ||
| ochami boot bmc add -d \ |
There was a problem hiding this comment.
Same as line 28 with the -e flag unless I'm misunderstanding how the bulk add works.
There was a problem hiding this comment.
The new API requires a "name" to be passed for each item when using *Simple() functions.
In the generated CLI (which doesn't support bulk add), this is passed as a positional argument.
In the ochami CLI, it made sense to include this within the payload structure so that bulk items could be added without complicated positional argument handling. The current implementation puts that in metadata.name, since there's a field for that already in the metadata block. However, I can see the potential confusion of having to pass metadata without using -e, so I'm wondering what the best way of handling this is.
One way is to use a top-level name field in the simple API payload and note that in the documentation. Then, for the advanced API, the metadata.name field would be used.
I don't see the above to be a problem, but open to thoughts.
There was a problem hiding this comment.
I guess that would work. I just expected -e for advanced and no flag without out and if the required metadata.name isn't there, then it would return an error.
There was a problem hiding this comment.
Yeah, name is required so perhaps adding it as a required, top-level field in the payload the CLI reads makes sense then.
|
I need to refactor this a bit due to some potential confusion arising from the current simple-vs-advanced API data structure (see https://github.com/OpenCHAMI/ochami/pull/131/changes/BASE..ee8d4d1a4457f7c7ee1fe2d1126bcc2fe5bfd303#r3676368580). |
2 participants
Description
Implement API changes from OpenCHAMI/fabrica#64.
Add
--envelopeflag to fabrica resource commands to allow the user to specify additional metadata that envelops the spec, which contains the actual data. By default (without--envelope), the payload is just the spec and additional metadata is ignored.Fixes #115
Checklist
make test(or equivalent) locally and all tests passgit commit -s) with my real name and email<filename>.licensesidecarLICENSES/directoryType of Change
For more info, see Contributing Guidelines.