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

implement sync_tcp_reconnect #101

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

kusma
Copy link
Member

@kusma kusma commented Aug 16, 2023

This is the sync_tcp_reconnect() proposal I made for #97. It's probably not perfect, but it seems to work.

This factor out the inner "connect to this specific address" part of
the current server_connect into its own function. As a nice side-benefit,
the connection loops now become simple enough to keep them separate,
making it much easier to read.
This allows us to reconnect to the same server without having to perform
hostname lookups etc again.
@kusma kusma changed the title WIP: sync_tcp_reconnect implement sync_tcp_reconnect Aug 16, 2023
@vcaputo
Copy link
Contributor

vcaputo commented Aug 17, 2023

I'm down with enabling a reconnect without name resolution in the existing tcp sockets implementation, so this PR conceptually seems like an improvement.

But I'd personally prefer having this API available on top of what's in this PR:
6a13f0c

I think it's a reasonable compromise, though I'm not super committed to the sync_use_socket() naming, it's not important to me.

@kusma kusma marked this pull request as draft August 18, 2023 08:25
@kusma
Copy link
Member Author

kusma commented Aug 18, 2023

I marked this as a draft until we've settled on a decision for #97, as it might get a bit in the way for how we want to do things there.

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.

None yet

2 participants