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.

Sign up for GitHub

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

fix(AIP-156): Do not define Update on singletons with only output only fields. #1188

Merged
merged 4 commits into from
Aug 4, 2023
Merged

fix(AIP-156): Do not define Update on singletons with only output only fields. #1188

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aded0e3
fix: Do not define Update on singletons with only output only fields.
lukesneeringer Jul 26, 2023
be0732a
changelog
lukesneeringer Jul 26, 2023
bb15813
fix: Fix typo.
lukesneeringer Jul 26, 2023
ecb2331
Merge branch 'master' into no-update-oo
toumorokoshi Aug 4, 2023
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
8 changes: 7 additions & 1 deletion aip/general/0156.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ always exist by virtue of the existence of its parent, with one and exactly one
per parent.

For example:

```proto
message Config {
option (google.api.resource) = {
Expand All @@ -32,6 +33,7 @@ message Config {
```

The `Config` singleton would have the following RPCs:

```proto
rpc GetConfig(GetConfigRequest) returns (Config) {
option (google.api.http) = {
Expand All @@ -57,12 +59,15 @@ rpc UpdateConfig(UpdateConfigRequest) returns (Config) {
- Singleton resources **should** define the [`Get`][aip-131] and
[`Update`][aip-134] methods, and **may** define custom methods as
appropriate.
- However, singleton resources **must not** define the [`Update`][aip-134]
Copy link
Contributor

@toumorokoshi toumorokoshi Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

nit: probably should move the update guidance to it's own line (e.g. singleton resources should define update unless all fields are output-only).

Although It seems like we could just refactor this guidance to just pointing to the update method AIP in general.

method if all fields on the resource are [output only][aip-203]
lukesneeringer marked this conversation as resolved.
Show resolved Hide resolved
- Singleton resources are always singular.
- Example: 'users/1234/thing'
- Example: `users/1234/thing`
- Singleton resources **may** parent other resources.

## Changelog

- **2023-07-26:** Clarified that read-only singletons should not have `Update`.
- **2021-11-02:** Added an example message and state parent eligibility.
- **2021-01-14:** Changed example from `settings` to `config` for clarity.

Expand All @@ -72,3 +77,4 @@ rpc UpdateConfig(UpdateConfigRequest) returns (Config) {
[aip-133]: ./0133.md
[aip-134]: ./0134.md
[aip-135]: ./0135.md
[aip-203]: ./0203.md#output-only