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.

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

[FS-1148] MLS-specific changes to adding members #3304

Merged
merged 13 commits into from
May 26, 2023
Merged

Conversation

mdimjasevic
Copy link
Contributor

@mdimjasevic mdimjasevic commented May 19, 2023

This is a follow-up PR to PR #3282. There are MLS-specific changes.

Given that an add commit cannot succeed partially, the response is not extended with a failed_to_add field like in the Proteus case. Instead, a 503 error is thrown in presence of unreachable backends/users. For more details see https://wearezeta.atlassian.net/wiki/spaces/CORE/pages/809763799/Offline+backend+handling+-+analysis+of+adding+participants+to+an+MLS+conversation.

Tracked by https://wearezeta.atlassian.net/browse/FS-1148.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label May 19, 2023
@mdimjasevic mdimjasevic force-pushed the fs-1148/only-for-mls branch 3 times, most recently from 1ed472a to 855953f Compare May 25, 2023 09:00
@mdimjasevic mdimjasevic force-pushed the fs-1148/only-for-mls branch 2 times, most recently from 33e9c96 to 1a6617c Compare May 26, 2023 08:47
@mdimjasevic mdimjasevic marked this pull request as ready for review May 26, 2023 08:55
In MLS a commit is rejected anyway so there is no point in passing
through FailedToProcess. Instead, a federator error is thrown if there
are unreachable backends when submitting an add commit.
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

LGTM

@mdimjasevic mdimjasevic merged commit e90ea40 into mls May 26, 2023
@mdimjasevic mdimjasevic deleted the fs-1148/only-for-mls branch May 26, 2023 14:22
mdimjasevic added a commit that referenced this pull request May 26, 2023
mdimjasevic added a commit that referenced this pull request May 29, 2023
mdimjasevic added a commit that referenced this pull request May 29, 2023
mdimjasevic added a commit that referenced this pull request May 29, 2023
mdimjasevic added a commit that referenced this pull request May 29, 2023
mdimjasevic added a commit that referenced this pull request May 29, 2023
mdimjasevic added a commit that referenced this pull request Jun 2, 2023
supersven pushed a commit that referenced this pull request Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants