Skip to content

Fix FTP upload file handle leak, correct deploy warning typo, and add rsync config test - #8

Open
zhang787jun wants to merge 1 commit into
masterfrom
codex-iqk8p0
Open

Fix FTP upload file handle leak, correct deploy warning typo, and add rsync config test#8
zhang787jun wants to merge 1 commit into
masterfrom
codex-iqk8p0

Conversation

@zhang787jun

Copy link
Copy Markdown
Owner

Motivation

  • Prevent leaking file descriptors during FTP deploy by ensuring uploaded files are closed after use.
  • Correct a spelling mistake in the runtime warning to make messages clearer for users and maintainers.
  • Add a unit test for get_rsync_configs to increase confidence in deploy configuration selection.

Description

  • Updated deploy_ftp in fabfile.py to open files with a context manager (with open(..., 'rb') as upload_file:) before calling ftp.storbinary so each file is closed after upload.
  • Fixed the unsupported deploy type message in fabfile.py to use not support {0} deploy method instead of the misspelled version.
  • Added tests/test_fabfile.py which stubs optional dependencies (fabric and simiki) to allow importing fabfile, and includes a pytest test that patches fabfile.configs and asserts get_rsync_configs() returns the rsync entry.

Testing

  • Ran pytest -q in the repository root and the test suite passed: 1 passed in 0.34s.
  • Verified the modified deploy_ftp logic via static inspection and ensured the repository imports succeed in the test environment by stubbing optional modules.

Codex Task

@ghost

ghost commented Feb 24, 2026

Copy link
Copy Markdown

Rooviewer Clock   See task

Review completed. No issues found. The changes correctly fix the file handle leak using a context manager, correct the typo in the deploy warning message, and add appropriate test coverage for the rsync config function.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant