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.

Sign up for GitHub

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

Jump to bottom

FTPS support #449

Merged
merged 9 commits into from
Jan 31, 2021
Merged

FTPS support #449

merged 9 commits into from
Jan 31, 2021

Conversation

odgalvin
Copy link
Contributor

@odgalvin odgalvin commented Jan 31, 2021 •
edited
Loading

Type of changes

  • New feature

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @PyFilesystem/maintainers may be pedantic in the code review.

Description

Added FTPS (FTP over TLS) support to FTPFS. Addresses #437.

  • Try to import FTP_TLS from the standard library, but if it's not available just default to a plain FTP connection, and set the tls attribute to False
  • Add a boolean tls argument to the constructor, and as an attribute
  • Add details and examples to the class docstring
  • Show the ftps:// scheme in the URL when TLS is enabled
  • Enable TLS when the ftps:// scheme is used in open/open_fs
  • Add basic test

@odgalvin
FTPFS: Add FTPS support.
515ca26 
Try to import FTP_TLS while still supporting older Python versions, add a 'tls' option to the FTPFS class, and show the 'ftps://' scheme in the repr when enabled.
@odgalvin
FTPFS: Add FTPS support to the FTPFS opener.
e19819b 
Enable TLS when using the ftps:// protocol
@odgalvin
FTPFS: Add FTPS check to test case.
2abeac8
@odgalvin
FTPFS: Fix opener test, and add extra test for the FTPS opener.
5d145cc
@althonos althonos self-assigned this Jan 31, 2021
@althonos althonos added this to the v2.4.13 milestone Jan 31, 2021
althonos
althonos requested changes Jan 31, 2021
View reviewed changes
Copy link
Member

@althonos althonos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for taking the time to implement this! I just have a change request for platform where FTP_TLS is not available, and we should be in the clear.

fs/ftpfs.py Show resolved Hide resolved
fs/ftpfs.py Outdated Show resolved Hide resolved
@odgalvin
Copy link
Contributor Author

Good idea! I just pushed a commit with that changed.

@althonos
Copy link
Member

althonos commented Jan 31, 2021 •
edited
Loading

Great! Now all that's left is to add a few # type: ignore comments to fs/ftpfs.py to make the linter happy, and to update the CONTRIBUTORS.md and CHANGELOG.md files 😉
(No worries about the coverage)

@coveralls
Copy link

coveralls commented Jan 31, 2021 •
edited
Loading

Coverage Status

Coverage decreased (-0.05%) to 95.322% when pulling e2c8bec on odgalvin:ftps_support into 8784fd6 on PyFilesystem:master.

@althonos althonos merged commit 1c68499 into PyFilesystem:master Jan 31, 2021
@althonos
Copy link
Member

Many thanks!

@althonos althonos linked an issue Jan 31, 2021 that may be closed by this pull request
FTPS #437
Closed
@odgalvin
Copy link
Contributor Author

No problem, thanks!

@odgalvin odgalvin deleted the ftps_support branch January 31, 2021 19:51
@Secret158
Copy link

You are the best

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@althonos althonos althonos approved these changes

Assignees

@althonos althonos

Labels
enhancement
Projects
None yet
Milestone
v2.4.13
Development

Successfully merging this pull request may close these issues.

FTPS
4 participants
@odgalvin @althonos @coveralls @Secret158