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.

Sign up for GitHub

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

Jump to bottom

Factor out SrvResolver #2

Open
DMRobertson opened this issue Nov 24, 2021 · 4 comments
Open

Factor out SrvResolver #2

DMRobertson opened this issue Nov 24, 2021 · 4 comments

Comments

@DMRobertson
Copy link
Contributor

Sydent has an SrvResolver class here: https://github.com/matrix-org/sydent/blob/8331f94e1e22c72c84683c271142f3237c707a27/sydent/http/srvresolver.py#L104

Synapse has one too with an associated test case.

I couldn't see one in Sygnal.
@DMRobertson
Copy link
Contributor Author

I think the entire srv_resolver file is duplicated in both cases.

This was referenced Nov 24, 2021
Open
Merged
@clokep
Copy link
Member

clokep commented Nov 29, 2021

I think the entire srv_resolver file is duplicated in both cases.

I believe this is the case (or meant to be the case). 😄

@DMRobertson
Copy link
Contributor Author

I looked at this briefly today, but was dismayed to see that Synapse's implementation includes make_deferred_awaitable which exists to interact with the logcontext machinery.

We could pull the log context into matrix python common. I would advocate pretty strongly against that. That feels like inappropriate coupling between "make this HTTP request" and "oh, by the way we are tracking CPU time".

@DMRobertson
Copy link
Contributor Author

See also matrix-org/synapse#10342.

@DMRobertson DMRobertson mentioned this issue Jan 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@clokep @DMRobertson