-
Notifications
You must be signed in to change notification settings - Fork 423
feat: Allow specifying SSL/TLS protocol version for FTP-TLS connections. #1958
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
feat: Allow specifying SSL/TLS protocol version for FTP-TLS connections. #1958
Conversation
|
Thank you for contributing. I expect to be able to look through this shortly. |
|
This all looks very nice - thank you. One question: why have the extra argument, when we already have one (tls=). Since the existing one is bool, it could still allow for all the str options you are adding here. |
|
Hello @martindurant in this apporach I just was ensuring not to break the current implementation, and yes it makes sense to reuse, I'm going to do a proposal here: 541f7f4 I will squash the commits if we move it forward |
|
We normally squash-merge anyway, so don't worry about that. This is ready for me to look at again? |
541f7f4 to
d022539
Compare
|
Comment attended, ready to review. |
martindurant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good. I have one request for you to consider.
b11601c to
47d1d27
Compare
|
Looks like the workflow approval got rejected when I squash + force push the commits. |
|
I believe approval is required every time if you are not already a contributor. For future reference, we squash-merge in this repo anyway, so no need to consolidate your commits in the PR branch. |
This change introduces support for implicit FTPS connections in the FTPFileSystem. Previously, only explicit FTPS was supported.
A new parameter security tls has been added to the FTPFileSystem constructor. This parameter allows users to specify the SSL/TLS protocol to be used for implicit FTPS connections,
such as TLSv1.2.
Key changes:
This enhancement provides greater flexibility and compatibility for connecting to various FTP servers requiring implicit FTPS with specific security protocols.