-
Notifications
You must be signed in to change notification settings - Fork 66
[specs] update arm LRO test to correspond to mockapi #3506
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
base: main
Are you sure you want to change the base?
Conversation
commit: |
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this scenario. Generally speaking, the POST response will never contain the final result, this is only returned after polling. We include the final result essentially just for swagger processors that don't understand x-ms-long-running-operation-options.
Having the poller return a 202 is not standard, but might be part of the required handling. In any case, void is not a final result, so it seems wrong to include it in the 'fake' 200 responses of the POST endpoint
markcowl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the previous is actually the way this API is represented in most async APIs that only return cancel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem may be on the mockapi.
@XiaofeiCao Can you check about live service response of action LRO with Location header? Do them typically have a body or not?
On the other hand, I don't think the initial response would be the form of CancelResult model (if it has a body) -- as mentioned by Mark as well.
|
Searched in my memory for this kind of operation, found one and it happened to behave exactly this way, where the initial response body is typespec-autorest is ok with this too, see playground. Not sure if there's real case that initial response body is not null while different from final response body schema, but I agree with Mark as well. |
|
I agree that there's value in having pollers work with non-conformant services when there's enough information to make unambiguous, forward progress. However, there was no indication here that this was the expectation. I suggest that we add a new category of LROs in the Spector tests to cover these cases. The "pedantic" LROs follow the tsp/guidelines to the letter, and the "lenient" ones (like this one) do not, but have enough info that pollers can make forward progress. If this is too disruptive, then at minimum, we must document cases where the mock deviates from the spec and there's an expectation that SDKs are able to cope with it. |
|
Hi @jhendrixMSFT , the guideline you mentioned in your issue is for data-plane. "202 + IMHO, one should not rely on the response body even if it's returned, since I don't see any schema enforcement on the response body schema. Some service even return the resource itself, see: |
|
Hmm that seems to contradict the vNext design guidelines (I'm familiar with the one you cited). Which one is correct?
Can you clarify what you mean here? The Rust poller expects the specified status monitor schema to be returned since that's what's authored. |
The Spector test cases under azure/resource-manager are for management-plane, hence the ARM guideline should be followed here. The vNext design guideline you mentioned is for data-plane. You can tell from its introduction:
By this I mean under ARM guideline, |
|
I thought the POR was to unify the LRO rules? But we can ignore that as it's a larger discussion. What I'm struggling with is that the tsp says a
While it's a reasonable assumption, to be pedantic, the spec is unclear here. It does not make any mention of a body, or absence thereof. We should follow up with the ARM team to have them clarify the spec. |
|
Hi Joel, I don't think the The standard flow(for LRO POST) is basically:
Or if it completes on initial request:
|
|
Thanks this helps. I think this can be closed and we'll follow up on our end. |



This test is causing issues for the Rust team because the spec specifies that the response will be a
CancelResult, while the initial response isvoid. We think this test is trying to go for specific ARM behavior, as such, we are not changing the mockapi response, instead updating the spec to reflect what the mockapi returns