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

Switch to sync code (again) #418

Closed
leplatrem opened this issue Apr 18, 2024 · 2 comments · Fixed by #430
Closed

Switch to sync code (again) #418

leplatrem opened this issue Apr 18, 2024 · 2 comments · Fixed by #430
Labels
enhancement New feature or request

Comments

@leplatrem
Copy link
Collaborator

Originally, the client was written using sync code.

It was then switch to async in #158 for a project that was killed in the mean time.

Since using a sync API is a requirement to be integrated in the application-services monorepo, let's revert and switch back to sync.

See Also

@leplatrem leplatrem added the enhancement New feature or request label Apr 18, 2024
@bendk
Copy link

bendk commented Jun 21, 2024

I've been thinking about this one a bit more and maybe it's not necessary.

We can always use something like pollster to block on an async function which lets us adapt the async interface to be sync. This should allow current RS consumers to use the client without changing too much code.

The other issue is that some consumers don't want app-services components to startup threads. I can see 2 solutions for this:

  • Implement dependencies like Requestor using methods that are formally async, but actually make blocking calls. This is kind-of hacky, but should work fine in the case where we're just going to block anyway using pollster.
  • Implement dependencies using something like fairybridge which handles async using the foreign language's runtime. This would allow the client to expose both a sync and async interface.

I'm going to try to test this out in an AS branch using a wrapper around the RS client. If this works, then we could consider bringing the client into app-services. Although, maybe a wrapper is fine -- especially since the app-services code is going to be moved into moz-central at some point.

@leplatrem
Copy link
Collaborator Author

Great! Thanks for coming back to this 😊

Looking at #158, it shouldn't be too much work to revert to sync methods, since the codebase hasn't changed significantly. I will also start a branch with that work, to see how it looks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants