Skip to content
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

Async Polling Standards #70

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

travisgosselin
Copy link
Member

These API Standards are a proposal for async polling integrated wtih RESTful-like endpoints. There are fundemantally two approaches I've struggled with in this:

  1. Integrating the resource to be async implicitly, and the state of the status monitor is part of the resource (job and status monitor are same object).
  2. Having a separate status monitor (not detailed in the PR) for the endpoint to have a pure resource representation (i.e. this has a representation of a status monitor object separate from the resource your trying to create). -> https://github.com/microsoft/api-guidelines/blob/vNext/azure/ConsiderationsForServiceDesign.md#long-running-operations

Further discussion and collaboration on if one of these is good enough or if we need both, etc. Thus far, it would seem SPS has started to use more of the first option from what I have seen.

This is draft and still under collaboration with internal working groups. Please do not waste time reviewing just yet.

"createdBy": "" | null, // optional
"completedDateTime": "" | null, // required
...EXTENDED...
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would be the response body of the status monitor or the object to be extended where status monitor and resource are combined.

"id": "f7cf8412-08ed-40c9-ac1b-296da9d1d970",
"ref": "sps:report::f7cf8412-08ed-40c9-ac1b-296da9d1d970",
"status": "RUNNING",
"detail": "same as detail from bulk... maybe error message? or complex object?",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same questions on if details on errors should be some standard error format? definitely this answer should match between this and bulk updates.


All asynchronous endpoint requests start with initiation. Since long-running requests need to be processed on background, asynchronous requests should initiate those processes with necessary input parameters. Background process details should be returned.

- Asynchronous requests **MUST** be a `POST` method, and should be represented as a resource, not an action-style endpoint.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will we allow async on DELETE? if so, it doesn't work with the pattern where status monitor and resource are the same object.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why DELETE conflict with status monitor? I think background job can be cancelled, but not deleted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current pattern the status monitor and the resource are the same. So you'd have to make a GET request to the resource with a given id... and poll until you get a 404 on it?

There are couple ways to accomplish asynchronous communication in REST APIs and those are not mutually exclusive, including polling, websockets and webhooks.

```note
At this time only asynchronous communication through polling are provided in the API Standards.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scoping this to just polling right now.

- Asynchronous requests **MUST** return 202 (Accepted) response status code when a long-running processing requests was successfully initiated.
- No other 2xx codes **SHOULD** be returned in response to the asynchronous request initialization, even if request has completed before the initiating request returns.
- `Operation-Id` header is **MAY** be supported and can be provided as part of the request.
- Process ID **SHOULD** be automatically generated and returned to the client if `Operation-Id` header is not provided.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably worth calling out that newly generated process id will be returned in the same Operation-Id header it it wasn't specified by client in request

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I struggled on the reason why we would want the Client/Consumer to ever specify the operation-id... and why not we always create and return one from the API?

- Asynchronous requests **MUST** return error details as part of completed long-running process response (other than request validation failures).
- Asynchronous requests **MUST** return 202 (Accepted) response status code when a long-running processing requests was successfully initiated.
- No other 2xx codes **SHOULD** be returned in response to the asynchronous request initialization, even if request has completed before the initiating request returns.
- `Operation-Id` header is **MAY** be supported and can be provided as part of the request.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What type is Operation-Id.. I assume it's an opaque string, but double checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants