-
Notifications
You must be signed in to change notification settings - Fork 325
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
Versioned combinators and access_role changes in V3 #2841
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
582b873
Add Versioned combinators and newtype
pcapriotti 647e5bc
Change access roles in V3 API
pcapriotti db082be
Rename AccessRoleV2 to AccessRole
pcapriotti 5f9e3b0
More V3 endpoint changes
pcapriotti 5c35910
Fix golden tests
pcapriotti be3af4c
Version conversation listing endpoint
pcapriotti 1b6dc79
Revert access update schema to V2
pcapriotti 3c84f77
Document event versioning
pcapriotti 743add8
Use API v3 in tests for listing conversations
pcapriotti e7c2375
Fix version bound in conversation list endpoint
pcapriotti bb6308a
Use v3 access update API in tests
pcapriotti ac59330
Add example timeline in event versioning docs
pcapriotti da93213
Fix off-by-one errors in event versioning docs
pcapriotti a5e83f8
Use v3 conv pagination API in tests
pcapriotti f2b9074
Fix collision in versioned swagger schemas
pcapriotti 8d472ae
Add CHANGELOG entry
pcapriotti b0c843e
Fix assertions in brig related to self convs
pcapriotti b6c87d3
More integration test fixes
pcapriotti 40b994a
fixup! More integration test fixes
pcapriotti File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
- The endpoints `POST /conversations/list` and `GET /conversations` have been removed. Use `POST /conversations/list-ids` followed by `POST /conversations/list` instead. | ||
- The endpoint `PUT /conversations/:id/access` has been removed. Use its qualified counterpart instead. | ||
- The field `access_role_v2` in the `Conversation` type, in the request body of `POST /conversations`, and in the request body of `PUT /conversations/:domain/:id/access` has been removed. Its content is now contained in the `access_role` field instead. It replaces the legacy access role, previously contained in the `access_role` field. | ||
- Clients implementing the V3 API must be prepared to handle a change in the format of the conversation.access_update event. Namely, the field access_role_v2 has become optional. When missing, its value is to be found in the field access_role. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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 very abstract and could benefit from concrete examples of a timeline including multiple backend and client releases, and at which point changes occur. The way I currently read this is "The backend can make incompatible event changes as long as the client can deal with both the old format and the new format". That just moves the burden of backwards compatibility from the server to the clients, which is not ideal as old versions of clients are around for much longer than old versions of the backend. What am I missing?
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.
Well, the
access_role
change in this PR is an example of change X.I added an example timeline. Not sure it adds anything useful, though, as it's mostly restating the same things with example numbers instead of N and Q.
The burden of backwards compatibility is shared with this system, and goes away after some time. At first both the backend and the clients have to support two formats. At some point in the future, one of the two can drop the old format. And finally the other one can drop it as well. In fact, the system is pretty symmetric wrt backend and clients.
I agree that it would be nicer to properly version events, but that seems to be quite a major undertaking, and might have some performance implications. This requires some extra bookkeeping, but ultimately achieves the same effect of getting rid of legacy code from all code bases, which is the ultimate goal anyway.