Automx fix #525
Open
Automx fix #525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
October 22, 2023 11:43
…script Added zipfile and urllib.request in 'postwhite.py' to improve the handling of archive downloads and extractions from specified repositories. Refactored 'install_from_archive' to increase the script's portability by removing the 'wget' command, improving reliability across different systems. Included robust error handling to improve user experience and problem diagnosis in case of download or extraction failures.
Modified archive extraction process in automx.py to include verification and error handling functionality. Now, the output of the unzip command is captured to confirm that the directory was successfully created. An exception is raised if the directory is not created, allowing early detection and diagnosis of potential issues.
Enhanced modoboa_installer/utils.py to now include two new methods, info and warning, to print messages in different colors. Also added a more robust download_remote_file function that downloads a file from a remote server with specified retries and exceptional download failure handling. This function was then used to replace previous ad-hoc download implementations for improved code reusability and readability.
…raction Modified the `utils.download_remote_file` function in `utils.py` to return the absolute path of the downloaded file instead of relative path. This change extends the robustness as it makes sure we are always unzipping the downloaded file even if cwd changes. Also, adjusted the `automx.py` script to use the absolute path returned from the download function when extracting the file. This ensures that the correct file gets extracted regardless of current working directory.
In utils.py, the wget command was updated in the exec_cmd function. Previously, the command did not specify the output file for the downloaded content. Now, a new target parameter has been added to the wget command to direct the download to a specific file. This change is done to provide more control over the file handling process within the function.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #525 +/- ##
==========================================
- Coverage 54.40% 53.73% -0.67%
==========================================
Files 10 10
Lines 761 776 +15
==========================================
+ Hits 414 417 +3
- Misses 347 359 +12
☔ View full report in Codecov by Sentry. |
Spitfireap
reviewed
Oct 26, 2023
| printcolor(message, BLUE) | ||
|
|
||
|
|
||
| def warning(message): |
There was a problem hiding this comment.
While you're at it, could change evey instance of utils.printcolor(*,YELLOW/BLUE) to these functions?
tonioo
reviewed
Oct 27, 2023
| @@ -67,3 +86,6 @@ def restore(self): | |||
| if os.path.isfile(postwhite_backup_configuration): | |||
| utils.copy_file(postwhite_backup_configuration, self.config_dir) | |||
| utils.success("postwhite.conf restored from backup") | |||
|
|
|||
| def download_file(self, url, destination): | |||
There was a problem hiding this comment.
Why do you create a new function to download a file using urllib instead of using the function you're adding in the utils.py file?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the issue/feature this PR addresses:
The
_setup_venvmethod in the existing codebase has been experiencing intermittent SSL connection issues when attempting to download a file usingwget. This problem has been causing the method to fail occasionally, preventing the successful setup of a virtual environment. Additionally, the method assumes the downloaded archive will be located in a certain directory without verifying the file's existence, leading to potential errors during the unzipping process.Current behavior before PR:
_setup_venvmethod useswgetto download a file, but if an SSL connection issue occurs, the method fails without retrying the download.Desired behavior after PR is merged:
download_remote_filehas been introduced to handle file downloads, with the capability to retry the download a specified number of times in case of failures, mitigating the impact of intermittent SSL connection issues._setup_venvmethod now utilizes thedownload_remote_filemethod to handle the file download, ensuring that the file exists before proceeding to unzip it.download_remote_filemethod ensures the target directory exists or creates it if necessary, and returns the absolute path of the downloaded file, providing more robust file handling and aiding in troubleshooting if issues arise in the future.