-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: AIP-214 – Resource expiration #39
base: main
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,39 @@ | |||
# Resource expiration | |||
|
|||
Customers often want to provide the time that a given resource or attribute of |
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.
- @lukesneeringer: Emphasize customer-provided expiry times.
Services **must** always return the expiration time in the `expire_time` field | ||
and leave the `ttl` field blank when retrieving the resource. | ||
|
||
Services that rely on the specific semantics of a "time to live" (e.g., DNS |
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.
- Discussion: What about if someone really needs a period string (like what is in AIP-142)?
- @lukesneeringer: What did we even say in AIP-142? (went to look)
- We said ints preferred, ISO 8601 period strings allowed.
- Discussion about what happens with a period string of one day that crosses DST.
- Consensus: Ints (seconds) are preferred, named
ttl_seconds
. If you really need period strings, specify that they are offset using UTC with daylight savings ignored. Minutes, hours are acceptable, anything larger (not fixed length) is advised against.- @lukesneeringer: Days should be fine, right?
- @jskeet: Daylight savings
- @lukesneeringer: I assert that when you use
foo_days
, you are measuring in numbers like 30, 60, 90, and probably DST does not matter anymore. - @jskeet: We should warn people, but I think I agree.
- Consensus: Ints preferred (seconds, days, hours, minutes), named accordingly. Period strings allowed. Offset UTC, DST ignored.
- @lukesneeringer: What did we even say in AIP-142? (went to look)
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.
Update AIP-142 similarly re: canonical unit being {days, hours, minutes, seconds}.
aip/general/0214/expiry.oas.yaml
Outdated
description: | | ||
The name of the book. | ||
Format: publishers/{publisher}/books/{book} | ||
expire_time: |
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.
- @rhamiltonsf: Why is this a property of the resource? Should it maybe be a header?
- The
Expires
header works well here. - @lukesneeringer: Works for reading. How do you specify?
- Seems like you could send an
Expires
header on PATCH/PUT.
- Seems like you could send an
- @jskeet: I want to challenge the assertion that the expire time is not a property of the resource. I defer to others in the room about HTTP knowledge, but I think the
Expires
header refers to the expiry of the response, not the expiry of the resource. - @rhamiltonsf: This is a good point. You have identified two different ways to use
Expires
.
- The
- Consensus: This AIP is not about messaging, it is about the resource itself, which will get expunged from the system. Property of the resource is correct, but we need to make it more clear what the AIP covers.
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.
Looks good. 👍
I changed the oas example to more consistent with the protobuf one.
No description provided.