Issue distilled from Matrix chat starting here.
In the external API in omicron, most endpoints with two versions present include an operation_id that allows the tooling to associate the two versions as versions of the same thing. instance_ephemeral_ip_detach does not, though, and it didn't seem to be a problem.
After some discussion, it sounds like this works because the old version's schema is already blessed, so that schema doesn't get regenerated (if it did, it would have the v12341234_ prefix in the operation ID). The apparent addition of a new endpoint is fine because the user ran generate to tell the tooling that it's fine.
I find it pretty confusing that this is fine. Looking at the endpoints file, you'd reasonably conclude that operation_id is required on these old versioned functions. If it's not required or only required sometimes, I'd rather we only do it when it's required, but I think it would be better to find a way to make it required.
This is related to a broader worry I have that the tooling seems to demand this workflow:
- Change the code
- Run
check
- Think about what
check is telling you the schema mismatch consists in
- Only run
generate once you agree, on reflection, that the expected changes are what you want
Historically, this is not how we have used the schema generator. What we have done is:
- Change the code
- Generate the schema. (If it fails, fix the code.)
- Make sure the schema looks right
So if the other workflow is now what's expected, that's a new muscle we need to try to build. My sense was that that is not really the intention with the new tooling, and what we really want is for generate to just do the right thing virtually all the time but error out when the situation requires disambiguation.
Issue distilled from Matrix chat starting here.
In the external API in omicron, most endpoints with two versions present include an
operation_idthat allows the tooling to associate the two versions as versions of the same thing.instance_ephemeral_ip_detachdoes not, though, and it didn't seem to be a problem.After some discussion, it sounds like this works because the old version's schema is already blessed, so that schema doesn't get regenerated (if it did, it would have the
v12341234_prefix in the operation ID). The apparent addition of a new endpoint is fine because the user rangenerateto tell the tooling that it's fine.I find it pretty confusing that this is fine. Looking at the endpoints file, you'd reasonably conclude that
operation_idis required on these old versioned functions. If it's not required or only required sometimes, I'd rather we only do it when it's required, but I think it would be better to find a way to make it required.This is related to a broader worry I have that the tooling seems to demand this workflow:
checkcheckis telling you the schema mismatch consists ingenerateonce you agree, on reflection, that the expected changes are what you wantHistorically, this is not how we have used the schema generator. What we have done is:
So if the other workflow is now what's expected, that's a new muscle we need to try to build. My sense was that that is not really the intention with the new tooling, and what we really want is for
generateto just do the right thing virtually all the time but error out when the situation requires disambiguation.