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

Proposal: improve api by exposing packager and transporter #56

Open
andig opened this issue Nov 6, 2022 · 4 comments · May be fixed by #70
Open

Proposal: improve api by exposing packager and transporter #56

andig opened this issue Nov 6, 2022 · 4 comments · May be fixed by #70

Comments

@andig
Copy link
Contributor

andig commented Nov 6, 2022

Currently, packers and transporters are closely coupled in the New*ClientHandler. This creates problems when sharing a single physical connection (part of the transporter) amongst multiple clients where each client talks to a different slave id (part of the packager). The current api make it impossible to implement race-free concurrent access.

While packager and transporter already have distinct purposes, the respective types cannot be created individually from outside the library as the types are not public. Using NewClient2 is cumbersome as the required packager/transporter types both need be created taking an address as parameter which is pointless for a packager.

This PR makes the packager/transporter types public and hence allows re-using the transporter while creating immutable packagers per slave id.

/cc @frzifus @gq0 @hsteidl could I kindly ask you to take a look at the attached PR? It is not finished but clearly demonstrates the approach. It exposes quite a bit of additional (though not su useful) api with the packager and transporter types. Maybe we can come up with a more trimmed approach, I'd be happy to update the PR with any suggestions.

Note: I like the approach from #70 much more as it is much less invasive and introduces very little additional api.

@andig andig changed the title Proposal: improve api Proposal: improve api by exposing packager and transporter Nov 6, 2022
@frzifus
Copy link
Contributor

frzifus commented Apr 10, 2023

Sure, but I probably won't make it this week.

@andig
Copy link
Contributor Author

andig commented Apr 10, 2023

Put up a second PR as alternative. Much smaller api surface- kinda like that one ;)

@andig
Copy link
Contributor Author

andig commented Apr 16, 2023

@frzifus friendly ping. Could I get your review on #70? #59 may be "cleaner" but is much more invasive. #70 is sweet and short.

@frzifus
Copy link
Contributor

frzifus commented Apr 16, 2023

i have put it on my to-do list. but i can't promise anything in the next 14 days - Iam sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants