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

Ability to set TCP keepalive to detect/closed dead connections #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eLvErDe
Copy link

@eLvErDe eLvErDe commented Oct 5, 2020

Hello,

I had trouble on a server having tones of dead connection to remote FTP server in CLOSED_WAIT state. I'm not sure what exactly caused that (I suspect the remote server had issues) but still, TCP keepalive mecanism is here to avoid this.
So here's a new kwarg to aioftp Client providing the ability to request TCP keepalive.

Regards, Adam.

@pohmelie
Copy link
Collaborator

pohmelie commented Oct 7, 2020

I see a better solutions here, make socket property for client, which will be available after connection made. So anyone can tune it as it wish.

@eLvErDe
Copy link
Author

eLvErDe commented Oct 7, 2020

I see a better solutions here, make socket property for client, which will be available after connection made. So anyone can tune it as it wish.

Except when using as context manager, this is why I did this that way

@pohmelie
Copy link
Collaborator

pohmelie commented Oct 7, 2020

You always can use your own context manager with socket tune after connect call. Current approach lacks of flexibility and is to strict. Some time later someone will want another socket option to tune.

@eLvErDe
Copy link
Author

eLvErDe commented Oct 7, 2020

You always can use your own context manager with socket tune after connect call. Current approach lacks of flexibility and is to strict. Some time later someone will want another socket option to tune.

Well it's your call anyway. For me these options are quite important so I like the idea of having them exposed as parameters with proper documentation

@pohmelie
Copy link
Collaborator

pohmelie commented Nov 7, 2020

Sorry for a delay. I've just released, that stream is a public client attribute. So you can use something like:

sock = client.stream.writer.get_extra_info("socket")
...

And I think there should not be second network connection abstraction like socket.

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.

2 participants