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

Should header retry-after use type string or integer? #4988

Open
mikeharder opened this issue Jul 5, 2024 · 1 comment
Open

Should header retry-after use type string or integer? #4988

mikeharder opened this issue Jul 5, 2024 · 1 comment

Comments

@mikeharder
Copy link
Member

mikeharder commented Jul 5, 2024

Schema for both autorest v2 and v3 requires header retry-after in examples to be type string instead of int32:

"retry-after": {
"type": "string",
"description": "An example value of the retry-after header sent by the server, if applicable in the example.",
"minLength": 1
}

"retry-after": {
"type": "string",
"description": "An example value of the retry-after header sent by the server, if applicable in the example.",
"minLength": 1
}

This seems to align with the HTTP spec:

The Retry-After field value can be either an HTTP-date or a number of seconds to delay after receiving the response.

Retry-After = HTTP-date / delay-seconds

A delay-seconds value is a non-negative decimal integer, representing time in seconds.

delay-seconds  = 1*DIGIT

Two examples of its use are

Retry-After: Fri, 31 Dec 1999 23:59:59 GMT
Retry-After: 120

In the latter example, the delay is 2 minutes.

https://www.rfc-editor.org/rfc/rfc9110#field.retry-after

However, existing examples (and at least some specs) in azure-rest-api-specs use a mix of string and int32:

https://github.com/search?q=repo%3AAzure%2Fazure-rest-api-specs+retry-after+language%3AJSON&type=code&l=JSON

Should the schema be updated to allow both string and int32 (if possible)? Or is it by design that all examples should use string, so we should ensure the example generator does this?

Example spec with issue:

"retry-after": {
  "description": "This header will only be present when the operation status is a non-terminal status. It indicates the minimum amount of time in seconds to wait before polling for operation status again.",
  "type": "integer",
  "format": "int32"
}

https://github.com/Azure/azure-rest-api-specs/blob/ab064e0047ec560a700d6b501097d99471ad817b/specification/communication/data-plane/Email/preview/2023-01-15-preview/CommunicationServicesEmail.json#L136-L140

Azure/azure-rest-api-specs#29699

Teams Discussion: https://teams.microsoft.com/l/message/19:[email protected]/1720141321497?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=3e17dcb0-4257-4a30-b843-77f47f1d4121&parentMessageId=1720141321497&teamName=Azure%20SDK&channelName=API%20Spec%20Review&createdTime=1720141321497

@timotheeguerin?

@mikeharder mikeharder self-assigned this Jul 5, 2024
@mikeharder mikeharder changed the title Should header "retry-after" use type "string" or "int32"? Should header "retry-after" use type "string" or "integer"? Jul 6, 2024
@mikeharder mikeharder changed the title Should header "retry-after" use type "string" or "integer"? Should header "retry-after" use type "string" or integer? Jul 6, 2024
@mikeharder mikeharder changed the title Should header "retry-after" use type "string" or integer? Should header retry-after use type string or integer? Jul 6, 2024
@mikekistler
Copy link
Member

We always want the value of retry-after to be "delay-seconds" and not a date, and RFC 7231 says:

A delay-seconds value is a non-negative decimal integer, representing time in seconds.

So integer is the appropriate type for this header value.

Furthermore, it is common that string and integer values are serialized in headers in the same way, so it might be possible to change the API description from type: string to type: integer in a non-breaking way (i.e. without changing the behavior of the service). As long as the service does not wrap the value in quotes, e.g.

Retry-after: "180"

but most services will not do this so it might be fine.

Bottom line: Our tooling should allow Retry-After to be defined as either type: integer (preferred) or type: string.

@mikeharder mikeharder removed their assignment Jul 10, 2024
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

No branches or pull requests

2 participants