Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Added v3.0 of the Azure REST API Guidelines #191
Added v3.0 of the Azure REST API Guidelines #191
Changes from all commits
c3033b0
8231bb4
f89d062
8177038
0aacf3b
ab075b2
8616ddb
de02fdd
0b1eb6b
92a6c1f
e7e80e3
206a874
4a19fe8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 wish this section started with a strong statement describing why compatibility is important to gaining trust with enterprise customers and how we will be striving for 100% compact. Right now, it reads like a prescription for trivial breaking changes, like removing a "foo" field. If you want to use an example of a breaking change, I would suggest examples related to closing a security hole, privacy issue, or geo-political issue, which by the way are the only API breaking changes we should consider. These changes improve trust we build with customers, and so they offset the loss of trust caused by breaks.
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.
Agreed, We have such guidance currently in another doc, and want to merge that either here or up one level in the Microsoft guidelines in a future PR.
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.
In reading this the first time, I agreed - we need a more meta "what will be considered for a breaking change" and then anything else is automatically not a breaking change. I think that wording can be a follow-on after an API review board meeting.
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. I believe this document was originally written specifically for resource providers/RPC. Which both means that it was more specific than general guidance (both based on specific issues that we wanted to prevent before a team even came to the board as well as things that are extra problematic for resource providers). In other words, it was a collection of case law rather than a holistic document.
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 seems to be increasingly at-odds with standard web development practices. I'm not sure how long Azure will be able to hold the line on this.
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 certainly a topic that can be visited during the update meetings that are planned.
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.
The Graph SDKs are able to avoid this problem because unknown properties retrieved from the server are round tripped. This follows the guidelines defined by the Atom publishing protocol. It is worth mentioning that Microsoft Graph does not make extensive use of PUT.
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 a consequence of https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#61-ignore-rule
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.
And please note that we've had instances where a service would reject a call with a read-only property that a client didn't know, but still tried to round-trip. The requirement for client behavior has to be in lock step with how a service is to treat unknown or read-only properties.
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 ARM/RPC specific (based on the use of the term "resource type")
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 implies that the list of breaking changes above is exhaustive. It is not.
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 we want to completely restructure the breaking change section as a first order of business .
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.
would you really want teams to do this to their customers?
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 wouldn't :-) I'll add this to the list of things to discuss with the board.
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.
Anecdotally, we've shipped client libraries before the service was publicly available (it had not left private preview). It has not ended well. We are definitely shipping libraries before the api version is available in all clouds, however (stack being the prime example of where it will never be possible to be 100% in lock step with public azure)
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.
As a random point, I really like this section and the features it describes. Are there libraries to help with this, or is it all an exercise to the reader? This seems like something that deserves a speclet (or even a DRAFT RFC!) on it's own.
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 wrote a javascript library for this a couple of years ago, but never got around to putting it into practice. The reality is that I locked the code down to a specific API and then depended on the contract not changing.
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.
@cleemullins, are you asking about libraries to help the service implement the behavior, or for guidance/libraries that help clients dynamically discover and light up features based on version availability?
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 it still a minimum of one year?
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'd love to get clarification on this time frame - it comes up often.
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.
Right now, I don't think the guidance of one year has officially changed. @johngossman - do you have any insight here?
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.
link
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.
Ref: @johngossman Is there a link I can link to for internal teams here?