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

Extend cross-room backfill request to work with different impls #1121

Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Aug 18, 2021

Servers should not be obliged to send a state_ids request if it already has the auth_events of a given event.

based on #1126

@@ -338,7 +338,6 @@ sub mk_await_request_pair

return $self->{$okey}{$ikey} = Future->new
->on_cancel( sub {
warn "Cancelling unused $shortname await for @paramvalues";
Copy link
Member Author

Choose a reason for hiding this comment

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

this was required because we now cancel the await_request_state_ids request if it is unused. That's normal behaviour, so we shouldn't warn on it.

@richvdh richvdh requested review from a team and removed request for a team August 23, 2021 17:08
@richvdh
Copy link
Member Author

richvdh commented Aug 24, 2021

this seems to be consistently failing CI with a test that isn't normally flaky. will take a closer look.

@richvdh richvdh requested a review from a team August 25, 2021 17:24
Servers should not be *obliged* to send a `state_ids` request if it already has
the `auth_events` of a given event.
@richvdh richvdh force-pushed the rav/unpick_update_auth_events_and_context_for_auth/4 branch from 9566118 to 9766edd Compare August 25, 2021 18:25
@richvdh
Copy link
Member Author

richvdh commented Aug 25, 2021

[the flaky test was in fact flaky, fixed in #1126]

@richvdh richvdh merged commit e51d4ba into develop Aug 26, 2021
@richvdh richvdh deleted the rav/unpick_update_auth_events_and_context_for_auth/4 branch August 26, 2021 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants