Skip to content

feat: support use with statement to create a ftpserver temporarily#567

Open
ramwin wants to merge 2 commits into
giampaolo:masterfrom
ramwin:master
Open

feat: support use with statement to create a ftpserver temporarily#567
ramwin wants to merge 2 commits into
giampaolo:masterfrom
ramwin:master

Conversation

@ramwin

@ramwin ramwin commented Jan 21, 2022

Copy link
Copy Markdown

No description provided.

@ramwin

ramwin commented Jan 22, 2022

Copy link
Copy Markdown
Author

@giampaolo the continuous-integration test failed. But the log shows that this is caused by pyftpdlib.test.test_servers.TestCornerCasesThreadMixin.test_quick_connect . I think it's not caused by my code. Could you review this pull request when you have spare time?

@ramwin

ramwin commented Jan 25, 2022

Copy link
Copy Markdown
Author

@giampaolo , Please review this pull request when you have any free time. Thanks a lot.

@gliptak

gliptak commented Dec 6, 2022

Copy link
Copy Markdown
Contributor

consider rerunning with current CI

Signed-off-by: Xiang Wang <ramwin@qq.com>
@ramwin

ramwin commented Dec 12, 2022

Copy link
Copy Markdown
Author

Hello, @gliptak . I rebase my branch onto the master branch. Review this pull request again when you are free.

@giampaolo

giampaolo commented Dec 12, 2022

Copy link
Copy Markdown
Owner

I don't think this is necessary (or at least not in this form). Do you need this to have a test FTP server to connect to during unit tests? If that's the case, then I think it may make more sense to add __enter__ / __exit__ to the test FTP server class:

class ThreadedTestFTPd(threading.Thread):

Comment thread pyftpdlib/servers.py
return

def __exit__(self, exc_type, exc_value, traceback):
logger.debug("you are existing FTPServerContext")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo: existing -> exiting

@ramwin

ramwin commented Dec 13, 2022

Copy link
Copy Markdown
Author

I have a simple script that need to start a ftpserver and call other web service to connect the ftpserver. It was inconvinent to first start a ftpserver and than run the simple script. I want to achieve this using with statement

with FtpServer(ftpusername, ftppassword) as tmp_ftpserver:
    requests.post("http://otherserver/download", json={
        ftpusername, ftppassword, filepath
    })

after the script, the ftpserver can be closed automatically.

@ramwin

ramwin commented Dec 13, 2022

Copy link
Copy Markdown
Author

Other wise, I will have to open two terminal.

First terminal

python -m ftpdlib --username=admin --password

Second terminal

python3 main.py

and I can only stop the ftpserver after the main script finished.

@giampaolo

Copy link
Copy Markdown
Owner

I forgot I added it, but FTPServer class already supports the context manager protocol:
327f3e0
That is different than your context manager though: on __enter__ it does NOT automatically call server_forever() in a thread.

@ramwin

ramwin commented Dec 15, 2022

Copy link
Copy Markdown
Author

I forgot I added it, but FTPServer class already supports the context manager protocol: 327f3e0 That is different than your context manager though: on __enter__ it does NOT automatically call server_forever() in a thread.

The function automatically call server_forever() in a thread is what I really want. That's why I create the FTPServerContext. I want to create a ftpserver temporally and use it in afterword scripts.

@ramwin

ramwin commented Dec 15, 2022

Copy link
Copy Markdown
Author

I tried use the blocking=False parameter, It didn't work as I wanted. My code looks like this:

with FTPServer(("localhost", 2121), handler) as f:
    f.serve_forever(blocking=False)
    print("start success")
    with FTP() as ftp:
        ftp.connect(host='localhost', port=2121)
        ftp.login()
        print(ftp.dir())

there is no start success printed on the screen, neither will the later code been executed.

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.

4 participants