Skip to content
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

New module for uploading to the SFTP the results of a batch #433

Merged
merged 17 commits into from
Mar 17, 2025

Conversation

jaimeozaez
Copy link
Member

@jaimeozaez jaimeozaez commented Mar 14, 2025

This PR introduces the upload_results module, which automates the process of uploading batch results to their respective COD folders. The module performs the following tasks:

  • Identifies the CODs processed in a given batch.
  • Compresses the analysis_results folder with password protection.
  • Uploads the compressed file to the SFTP server.
  • Sends an email notification to the laboratory, including details of the uploaded results and the password to access the compressed file.

@jaimeozaez jaimeozaez self-assigned this Mar 14, 2025
@jaimeozaez jaimeozaez marked this pull request as ready for review March 17, 2025 14:44
sftp = self.relecov_sftp.sftp

# Define the remote path where the file is to be uploaded
remote_dir = f"/{cod}/ANALYSIS_RESULTS"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this folder name ANALYSIS_RESULTS should be in the conf file?

Copy link
Contributor

@victor5lm victor5lm Mar 17, 2025

Choose a reason for hiding this comment

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

@saramonzon Do you think this should be added as a new field in configuration.json or as a new entry from sftp_handle, for example, as "analysis_results_folder": "ANALYSIS_RESULTS"?

"sftp_handle": {
"sftp_connection": {
"sftp_server": "sftpgenvigies.isciii.es",
"sftp_port": "22"
},
"metadata_processing": {
"sample_id_col": "Sample ID given for sequencing",
"header_flag": "CAMPO",
"excel_sheet": "METADATA_LAB",
"alternative_sheet": "5.Viral Characterisation and Se",
"alternative_flag": "CAMPO",
"alternative_sample_id_col": "Sample ID given for sequencing"
},
"abort_if_md5_mismatch": "False",
"platform_storage_folder": "/tmp/relecov",
"allowed_file_extensions": [
".fastq.gz",
".fastq",
".fq",
".fq.gz",
".fasta",
".fasta.gz",
".fa",
".fa.gz",
"bam"
],
"allowed_download_options": [
"download_only",
"download_clean",
"delete_only"
],
"skip_when_found": [
"#",
"Hash",
"Path"
]
},

Copy link
Member

Choose a reason for hiding this comment

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

I think it should be fine in sftp_handle :)


if not matching_cod:
stderr.print(f"[red]Batch {self.batch_id} was not found in any COD* folder.")
sys.exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

use raise and handle upstream

Copy link
Contributor

Choose a reason for hiding this comment

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

If I am not wrong this could be changed by:

if not matching_cod:
    raise FileNotFoundError(f"Batch {self.batch_id} was not found in any COD* folder.")

What do you mean by "handle upstream" though? @saramonzon

Copy link
Member

Choose a reason for hiding this comment

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

at some point (in the main.py) this raise must be handle so the user does not get an error but a error message, you can use the debug flag that @albatalavera introduced, so if debug you show the traceback from the raise, and if not you only show the message with the sys.exit("message") but in the main, no in a "inside code function"


if not template_path:
stderr.print("[red]Error: You must provide a template_path as an argument.")
sys.exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

@Shettland in the class init methods should we use also raise? Or here is like in main that it's fine to use it?

We may have a lot of sys.exit in init methods, not sure if in relecov-tools but on bu-isciii tools for sure

Copy link
Member

Choose a reason for hiding this comment

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

here let's discuss this with @Shettland in case we need to do the same here, in the case of the other function is clear, i'm not sure in this case

@saramonzon
Copy link
Member

Also pyzipper must be add to requirements.txt

@jaimeozaez jaimeozaez merged commit 38b7602 into BU-ISCIII:develop Mar 17, 2025
8 checks passed
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.

3 participants