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

MFMT Implementation #435

Merged
merged 16 commits into from Oct 9, 2017

Conversation

Projects
None yet
2 participants
@tahirijaz24
Contributor

tahirijaz24 commented Sep 22, 2017

Added the MFMT command in accordance with the draft (https://tools.ietf.org/html/draft-somers-ftp-mfxx-04).

Made the following changes:

  • exposed the utime method in the filesystem class
  • Added the 'T' permission for MFMT
@giampaolo

Overall this looks great but unit tests are missing. Do you think you can provide some?

Show outdated Hide outdated pyftpdlib/handlers.py
@tahirijaz24

This comment has been minimized.

Show comment
Hide comment
@tahirijaz24

tahirijaz24 Oct 6, 2017

Contributor

@giampaolo Thank you once again for your kind input. I have made the suggested changes, please let me know what you think :)

Contributor

tahirijaz24 commented Oct 6, 2017

@giampaolo Thank you once again for your kind input. I have made the suggested changes, please let me know what you think :)

replaced assertTrue with assertIn in MFMT testcases and instead of us…
…ing mdtm in test_mfmt testcase using os.path.getmtime to fetch file's last modified time.
@tahirijaz24

This comment has been minimized.

Show comment
Hide comment
@tahirijaz24

tahirijaz24 Oct 9, 2017

Contributor

Thank you so much @giampaolo, that is definitely a better way to do things. The MFMT testcases are now using assertIn instead and I am using os.path.getmtime to fetch the actual file's last modified time. I am extremely grateful for you to take out the time to go through my PR - this is a great learning experience for me. Please let me know if there is anything at all you would like me to change or do differently. Thank you once again for everything! btw I have been playing with psutil and its awesome!

Contributor

tahirijaz24 commented Oct 9, 2017

Thank you so much @giampaolo, that is definitely a better way to do things. The MFMT testcases are now using assertIn instead and I am using os.path.getmtime to fetch the actual file's last modified time. I am extremely grateful for you to take out the time to go through my PR - this is a great learning experience for me. Please let me know if there is anything at all you would like me to change or do differently. Thank you once again for everything! btw I have been playing with psutil and its awesome!

@giampaolo giampaolo merged commit 6941683 into giampaolo:master Oct 9, 2017

0 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
coverage/coveralls Coverage pending from Coveralls.io
Details
@giampaolo

This comment has been minimized.

Show comment
Hide comment
@giampaolo

giampaolo Oct 9, 2017

Owner

Overall great work! Thanks. I appreciate what you did as it's a useful enhancement. I hope you can contribute other PRs in the future. ;-)

Owner

giampaolo commented Oct 9, 2017

Overall great work! Thanks. I appreciate what you did as it's a useful enhancement. I hope you can contribute other PRs in the future. ;-)

giampaolo added a commit that referenced this pull request Oct 9, 2017

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