Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove legacy singular replication endpoint for resyncing remote user devices #14808

Closed
clokep opened this issue Jan 10, 2023 · 6 comments · Fixed by #15418
Closed

Remove legacy singular replication endpoint for resyncing remote user devices #14808

clokep opened this issue Jan 10, 2023 · 6 comments · Fixed by #15418
Labels
good first issue Good for newcomers O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@clokep
Copy link
Member

clokep commented Jan 10, 2023

I.e. all the code deprecated in #14716, this is pretty much:

  • user_device_resync methods (I think? I don't see a reason to have these that just call the multi_ version)
  • ReplicationUserDevicesResyncRestServlet

I think that's it. 👍

@clokep clokep added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. O-Uncommon Most users are unlikely to come across this or unexpected workflow good first issue Good for newcomers labels Jan 10, 2023
@clokep
Copy link
Member Author

clokep commented Jan 10, 2023

Since #14716 made it into the release branch I think we can pretty much do this whenever.

@tjay27
Copy link
Contributor

tjay27 commented Jan 11, 2023

in _multi_user_device_resync there is a use of ReplicationUserDevicesResyncRestServlet

# Fall back to single requests
result: Dict[str, Optional[JsonDict]] = {}
for user_id in user_ids:
result[user_id] = await self._user_device_resync_client(user_id=user_id)

Should we keep ReplicationUserDevicesResyncRestServlet ?

@clokep
Copy link
Member Author

clokep commented Jan 11, 2023

in _multi_user_device_resync there is a use of ReplicationUserDevicesResyncRestServlet

Should we keep ReplicationUserDevicesResyncRestServlet ?

No need to -- that's part of what we want to remove since it is the fallback path.

krish-bista added a commit to krish-bista/synapse that referenced this issue Jan 28, 2023
replacing the old device_resync with the new multi_ version of it.
@clokep clokep linked a pull request Jan 30, 2023 that will close this issue
4 tasks
@khandelwalarvind26
Copy link

Hey, I'm a beginner contributor to synapse and found this issue in the good first issues section. Since the PR for this issue hasn't been merged yet, can I work on this issue?

@tjay27
Copy link
Contributor

tjay27 commented Feb 6, 2023

@khandelwalarvind26
yes, go ahead :D

@skmanoj322
Copy link

Hey wanted to make my Frist open source contribution so am I in a right place if yes can I start working on this problem

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
4 participants