Skip to content

[18.0][MIG] storage_backend_ftp: Migration to 18.0#16

Closed
thienvh332 wants to merge 23 commits into18.0from
18.0-mig-storage_backend_ftp
Closed

[18.0][MIG] storage_backend_ftp: Migration to 18.0#16
thienvh332 wants to merge 23 commits into18.0from
18.0-mig-storage_backend_ftp

Conversation

@thienvh332
Copy link
Owner

@thienvh332 thienvh332 commented Apr 3, 2025

  • Added a timeout parameter (30 seconds) to the ftplib.FTP and ftplib.FTP_TLS initializations in the ftp context manager to address:

    [E8106(external-request-timeout)]. This ensures the FTP connection does not hang indefinitely in case of network issues.

acsonefho and others added 23 commits April 3, 2025 16:31
* list should return a python list of files.
* get should get and directly return the bynary from the FTP.
  Before, a string was being retrived and then encoded. This is
  problematic if the original binary was not utf-8.
Currently translated at 4.3% (1 of 23 strings)

Translation: storage-14.0/storage-14.0-storage_backend_ftp
Translate-URL: https://translation.odoo-community.org/projects/storage-14-0/storage-14-0-storage_backend_ftp/it/
Copy link

@nilshamerlinck nilshamerlinck left a comment

Choose a reason for hiding this comment

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

thanks, any chance to get a description that might explain why the timeout=30 change?

@thienvh332
Copy link
Owner Author

thanks, any chance to get a description that might explain why the timeout=30 change?

It comes from the pre-commit message below. Should I add it to the PR description?

[E8106(external-request-timeout), ftp] Use of external request method ftplib.FTP without timeout. It could wait for a long time

@nilshamerlinck
Copy link

yes, please, a reviewer might wonder why this change

Copy link

@nilshamerlinck nilshamerlinck left a comment

Choose a reason for hiding this comment

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

pre-approving

@thienvh332
Copy link
Owner Author

Updated PR description, Thanks sir 🙏

@thienvh332 thienvh332 closed this May 13, 2025
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.

10 participants