-
Notifications
You must be signed in to change notification settings - Fork 713
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.
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
base: master
Are you sure you want to change the base?
Conversation
Effectively, not all parts of all AIPs are relevant to all APIs. This section gives some examples of other considerations, although it's not intended to be exhaustive.
toumorokoshi
left a comment
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.
Thanks for starting this! I think we should do some more refining of the specific cases that are believed to be exceptional, and making sure that guidance is crystal clear to consumers.
I left a few comments so we can go back and forth on this.
| It doesn't make sense for *every* API to follow the AIPs - not even all | ||
| *Google* APIs. Some relevant factors to consider include: | ||
|
|
||
| - Who is writing code to call the APIs? Some AIPs (e.g. [141](AIP-141)) include |
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.
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).
| - Who is writing code to call the APIs? Some AIPs (e.g. [141](AIP-141)) include | ||
| 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 |
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.
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.
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.
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.
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.
Got it! Googlers, see b/217611342 for an example.
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.
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.
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.
We have some AIPs with specific scope, such as Cloud AIPs. It seems like we should refer to those categories.
| - 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 | ||
| 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 |
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.
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.
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.
+1. I would put it in a section on deprecation/removal.
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.
How about I move (and expand) details to the compatibility AIP, but link to it from here as an example of how context matters?
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.
definitely! I 100% support linking to give people a place to read more, but not duplicating guidance.
| that the API only needs to be called from Google-authored code, some of those | ||
| 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 |
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.
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.
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.
+1. I have no idea what case(s) this is alluding to.
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.
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.
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.
Ah, such as when all clients are in the monorepo?
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.
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.)
| - Who is writing code to call the APIs? Some AIPs (e.g. [141](AIP-141)) include | ||
| 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 |
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.
Is this referring to non-public APIs?
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.
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.)
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.
Ah, such as undocumented APIs purely for our web and mobile clients?
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.
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...
| that the API only needs to be called from Google-authored code, some of those | ||
| 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 |
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.
+1. I have no idea what case(s) this is alluding to.
| - 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 | ||
| 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 |
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.
+1. I would put it in a section on deprecation/removal.
|
Ping for more responses to comments, @toumorokoshi and @bgrant0607. (If we've gone off the idea of this PR at all, that's fine too. I'd just like to make progress in some sense.) |
|
Thanks! I left a couple more comments. I think there's just high-level philosophical agreement in some places - let's get that ironed out offline and we can round back here. There is definitely still value in defining the scope of AIPs. especially those that must adhere to other third-party specifications. |
|
|
||
| ## Scope | ||
|
|
||
| It doesn't make sense for *every* Google API to follow the AIPs. Some relevant |
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.
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).
|
Copying some high-level notes from the meeting today. We agree upon the following and doing some smaller changes to address the following:
|
Effectively, not all parts of all AIPs are relevant to all APIs. This section gives some examples of other considerations, although it's not intended to be exhaustive.