WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Added a section on AIP scope #1130

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

Open
jskeet wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added a section on AIP scope #1130

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc6a5e9
Added a section on AIP scope
jskeet Jun 9, 2023
675afb9
Address review comment around non-Google APIs
jskeet Jun 15, 2023
40524da
Added examples of third party APIs
jskeet Jul 5, 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
23 changes: 23 additions & 0 deletions aip/general/0001.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,27 @@ serve as the source of truth for API-related documentation at Google and the
means by which API teams discuss and come to consensus on API guidance. AIPs
are maintained as Markdown files in the [AIP GitHub repository][].

## Scope

It doesn't make sense for *every* Google API to follow the AIPs. Some relevant
Copy link
Contributor

@bgrant0607 bgrant0607 Jul 7, 2023

Choose a reason for hiding this comment

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

It may help to start with base assumptions about what types of APIs the AIPs are for: public APIs with a wide variety of external clients (examples are in https://google.aip.dev/9#client).

factors to consider include:

- Who is writing code to call the APIs? Some AIPs (e.g. [141](AIP-141)) include
Copy link
Contributor

@toumorokoshi toumorokoshi Jun 9, 2023

Choose a reason for hiding this comment

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

I think the wording of this section could be more succinct, with a clear litmus test of when the guidance applies or does not apply. We should use RFC-2119 must/should/should not to help clarify.

This section reads more like a conversation, which does not fit the style of the rest of the AIPs (which are itemized discrete guidance, sometimes with rationale attached).

requirements with an expectation of APIs being called from client libraries
written in diverse programming languages. If the API producer is confident
that the API only needs to be called from Google-authored code, some of those
Copy link
Contributor

@toumorokoshi toumorokoshi Jun 9, 2023

Choose a reason for hiding this comment

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

This wording is kind of vague and puts a lot of control in the hands of the API producer. This isn't really the case - despite the API producer's perspective, for example, Cloud has strict requirements for coverage in our clients to meet demand of our larger customers.

I think this goes back to my larger point about this section enumerating clear exceptional cases that allow a reader to easily bucket their APIs into a relevant category. There is always the fallback of reaching out to API design for questions.

Copy link
Contributor Author

@jskeet jskeet Jun 15, 2023

Choose a reason for hiding this comment

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

This isn't really the case - despite the API producer's perspective, for example, Cloud has strict requirements for coverage in our clients to meet demand of our larger customers.

Except there are Cloud APIs which do fall into this category. Annoyingly, I had an example of this just the other day... will try to find it.

Copy link
Contributor Author

@jskeet jskeet Jun 16, 2023

Choose a reason for hiding this comment

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

Got it! Googlers, see b/217611342 for an example.

Copy link
Contributor

@toumorokoshi toumorokoshi Jul 7, 2023

Choose a reason for hiding this comment

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

taking a brief look at the issue, I'd wonder if that's a classification issue.

Nevertheless I'm a bit concerned that there's statements in the AIPs that put a lot of control of which rules they follow in the AIP itself.

Maybe we can take this conversation offline? we won't resolve it offline I imagine.

Copy link
Contributor

@bgrant0607 bgrant0607 Jul 7, 2023

Choose a reason for hiding this comment

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

We have some AIPs with specific scope, such as Cloud AIPs. It seems like we should refer to those categories.

Copy link
Contributor

@bgrant0607 bgrant0607 Jun 9, 2023

Choose a reason for hiding this comment

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

Is this referring to non-public APIs?

Copy link
Contributor Author

@jskeet jskeet Jun 15, 2023

Choose a reason for hiding this comment

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

We have some APIs which are public, but not actually intended to be called by non-Google code. (In those cases the protos are published but we typically don't generate client libraries.)

Copy link
Contributor

@bgrant0607 bgrant0607 Jul 7, 2023

Choose a reason for hiding this comment

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

Ah, such as undocumented APIs purely for our web and mobile clients?

Copy link
Contributor Author

@jskeet jskeet Jul 11, 2023

Choose a reason for hiding this comment

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

Those would count as internal (I think) and not published. There are some that are expected to be called by agent processes on customers' GKE projects, as a slightly different example. I wish I had all the examples I've seen to hand...

requirements may not be relevant.
- How is the client code deployed? When the set of deployed clients is fully
known and fully controlled (most importantly for server-to-server APIs), many
Copy link
Contributor

@toumorokoshi toumorokoshi Jun 9, 2023

Choose a reason for hiding this comment

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

this is too vague, in my opinion: it's not clear what cases are exceptional, and I don't think service producers would know either.

I'm not confident that we should imply this level of freedom in adhering to the AIPs: there is significant value to Google overall in adhering to them.

It would be better to enumerate specific broader where non-adherence is acceptable (e.g. specific external API specifications that must be adhered to) and let ambiguity lead to dialogue instead.

Copy link
Contributor

@bgrant0607 bgrant0607 Jun 9, 2023

Choose a reason for hiding this comment

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

+1. I have no idea what case(s) this is alluding to.

Copy link
Contributor Author

@jskeet jskeet Jun 15, 2023

Choose a reason for hiding this comment

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

It's really for code which never leaves Google's own data centers - purely internal code where we always know what API versions callers are using. We suggest that internal APIs follow AIPs, but that makes a huge difference in terms of compatibility requirements.

Copy link
Contributor

@bgrant0607 bgrant0607 Jul 7, 2023

Choose a reason for hiding this comment

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

Ah, such as when all clients are in the monorepo?

Copy link
Contributor Author

@jskeet jskeet Jul 11, 2023

Choose a reason for hiding this comment

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

Yes, exactly that. (Or even if they're in multiple repos but it's a well-known and complete set of repos, it's still controllable.)

requirements around compatibility can be skipped. For example, if you
*absolutely know* that no code exists that will call an RPC, it's safe to
Copy link
Contributor

@toumorokoshi toumorokoshi Jun 9, 2023

Choose a reason for hiding this comment

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

I feel like this particular point would be better put into the backwards compatibility AIP. I've seen a few cases where this type of guidance pops up outside of the main AIP, and as a result it leads to only one section being edited and readers finding contradictions.

Copy link
Contributor

@bgrant0607 bgrant0607 Jun 9, 2023

Choose a reason for hiding this comment

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

+1. I would put it in a section on deprecation/removal.

Copy link
Contributor Author

@jskeet jskeet Jun 15, 2023

Choose a reason for hiding this comment

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

How about I move (and expand) details to the compatibility AIP, but link to it from here as an example of how context matters?

Copy link
Contributor

@toumorokoshi toumorokoshi Jul 7, 2023

Choose a reason for hiding this comment

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

definitely! I 100% support linking to give people a place to read more, but not duplicating guidance.

remove that RPC.
- Is the API actually implementing an existing API specification that can't be
changed? If so, AIP conformance is impossible and should be deemed out of
scope. Some such APIs may be REST-like but not follow AIP conventions (package
management APIs often fall into this category); others may not be HTTP-based
at all (e.g. ssh or database protocols).

## Types of AIPs

There are several different types of AIPs, described below. The list of AIP
Expand Down Expand Up @@ -269,6 +290,7 @@ state, and will link to the new, current AIP.

## Changelog

- **2023-06-09**: Added section on the scope of AIPs.
- **2023-05-10**: Updated names of current and editors and TLs.
- **2019-07-30**: Further clarified AIP quorum requirements.
- **2019-05-12**: Collapsed AIP approvers and editors into a single position,
Expand All @@ -286,3 +308,4 @@ state, and will link to the new, current AIP.
[@noahdietz]: https://github.com/noahdietz
[@shwoodard]: https://github.com/shwoodard
[@toumorokoshi]: https://github.com/toumorokoshi
[aip-122]: ./0141.md