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

feat(AIP-217): add brownfield adoption guidance #1385

Merged
merged 5 commits into from
Jul 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions aip/general/0217.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,97 @@ These facts lead to the following guidance:
provided during such an incident as described above, the parent or
resource **must** be included in the `unreachable` field.

### Adopting partial succcess

In order for an existing API that has a default behavior *differing* from the
aforementioned guidance i.e. the API call returns an error status instead of a
partial result, to adopt the `unreachable` pattern the API **must** do the
following:

* the default behavior **must** be retained to avoid incompatible behavioral
changes
* For example, if the default behavior is to return an error if any location
is unreachable, that default behavior **must** be retained.
* the request message **must** have a `bool return_partial_success` field
* the response message **must** have the standard
`repeated string unreachable` field
* the two aforementioned fields **must** be added simultaneously

When the `bool return_partial_success` field is set to `true` in a request, the
API **must** behave as described in the aforementioned guidance with regards to
populating the `repeated string unreachable` response field.

```proto
message ListBooksRequest {
// Standard List request fields...

// Setting this field to `true` will opt the request into returning the
// resources that are reachable, and into including the names of those that
// were unreachable in the [ListBooksResponse.unreachable] field. This can
// only be `true` when reading across collections e.g. when `parent` is set to
// `"projects/example/locations/-"`.
bool return_partial_success = 4;
}

message ListBooksResponse {
// Standard List Response fields...

// Unreachable resources. Populated when the request opts into
// `return_partial_success` and reading across collections e.g. when
// attempting to list all resources across all supported locations.
repeated string unreachable = 3;
}
```

#### Partial success granularity

If the `bool return_partial_success` field is set to `true` in a request that is
scoped beyond the supported granualirty of the API's ability to reasonably
report unreachable resources, the API **should** return an `INVALID_ARGUMENT`
error with details explaining the issue. For example, if the API only supports
`return_partial_success` when [Reading Across Collections][aip159], it returns
an `INVALID_ARGUMENT` error when given a request scoped to a specific parent
resource collection. The supported granularity **must** be documented on the
`return_partial_success` field.

## Rationale

### Using request field to opt-in

Introducing a new request field as means of opting into the partial success
behavior is the best way to communicate user intent while keeping the
default behavior backwards compatible. The alternative, changing the default
behavior with the introduction of the `unreachable` response field, presents
a backwards incompatible change. Users that previously expected failure when any
resource was unreachable, assume the successful response means all resources
are accounted for in the response.

### Introducing fields simultaneously

Introducing the request and response fields simultaneously is to prevent an
invalid intermediate state that is presented by only adding one or the other. If
only `unreachable` is added, then it could be assumed that it being empty means
all resources were returned when that may not be true. If only
`return_partial_success` is added, then the user wouldn't have a means of
knowing which resources were unreachable.

### Partial success granularity limitations

At a certain level of request scope granularity, an API is simply unable to
enumerate the resources that are unreachable. For example, global-only APIs may
be unable to provide granularity at a localized collection level. In such a
case, preemptively returning an error when `return_partial_success=true`
noahdietz marked this conversation as resolved.
Show resolved Hide resolved
protects the user from the risks of the alternative - expecting unreachable
resources if there was an issue, but not getting any, thus falsely assuming
everything was retrieved. This aligns with guidance herein that suggests failing
requests that cannot be fulfilled preemptively.

## Further reading

- For listing across collections, see [AIP-159][].

## Changelog

- **2024-07-19**: Add guidance for brownfield adoption of partial success.

[aip-159]: ./0159.md
Loading