Closed davidallendj closed 3 months ago
Cool! So as of this writing, parseRedfishPostDataV2()
isn't called by anything yet and we need to figure out how/when to call it over the old function, parseRedfishPostData()
. Since this is technically an incompatible API change, we probably want to roll it out slowly. There are a couple of ways that @davidallendj and I discussed doing this that we'd like discussion from the reviewers on:
--rfev2
) that tells SMD to use parseRedfishPostDataV2()
over parseRedfishPostData()
.parseRedfishPostDataV2()
to parse the POST request over parseRedfishPostData()
.For 1, all requests would use the new data format when SMD is run with the flag. For 2, the new format is only used if the header is set in the POST request. I'm not sure I see a use case for 2 to be able to include both formats except for debugging purposes so I lean more towards 1, but I am interested in seeing what the reviewers think.
Whichever method is chosen, @davidallendj and I agreed that introducing this change in a way that allows both functions to be used/interchanged and doesn't change the API (yet) is the way to introduce this change.
I suggest we explicitly version the data payload using jsonschema and then have the same endpoint read the version info and do the right thing.
@davidallendj I'm not sure what to do with this PR.
If you want us to merge it quickly so you can do a release, I recommend backing off the last commit so the behavior remains unchanged for the moment.
If you want to continue with implementation of RFD 39, I think this needs a few other pieces:
This just needs to be tested before merging now to make sure that it works.
Confirmed that the new parsing code works with the new magellan
data format using v0.1.3.
@davidallendj I'm not sure what to do with this PR.
If you want us to merge it quickly so you can do a release, I recommend backing off the last commit so the behavior remains unchanged for the moment.
If you want to continue with implementation of RFD 39, I think this needs a few other pieces:
- [ ] Import from the schemas repo so you aren't re-declaring these common structs.
- [x] Make the default behavior of SMD without the named/versioned schema to use the old internal schema for now. Updating the default should be considered a breaking change.
- [ ] Update the tests to ensure that clients get good failure information when a payload doesn't meet the schema definition.
- [ ] This will need to merge and release before Magellan can be changed to do the same thing unless we add feature flagging to Magellan now and change the default in the next version bump.
We're going to address each of these tasks in a new PR after this one gets merged. That way, we will have a version of SMD that is compatible with the new data format, and then we can make more improvements behind the scenes afterwards.
@davidallendj Is this ready to merge? I thought you were still working on it yesterday.
@alexlovelltroy Just waiting on @synackd to finish testing. Once he's done, we can merge.
This PR adds a new
parseRedfishPostDataV2()
function to parse a new format of data expected to be received frommagellan
. This function will achieve the same parity as the original parsing function and possible include more information when and if necessary.The function expects data in the following format: