Skip to content

Conversation

@ab604
Copy link
Contributor

@ab604 ab604 commented Oct 23, 2025

User description

Problem

The code in files.py was looking for a key file named aspera_tokenauth_id_rsa, but the actual key file in the repository is named asperaweb_id_dsa.openssh.

This mismatch caused authentication failures when users tried to download files using the Aspera protocol, resulting in password/passphrase prompts.

Solution

Updated both occurrences in files.py (lines ~9 and ~172) to use the correct key filename: asperaweb_id_dsa.openssh

Testing

  • Tested on Linux with pridepy download-file-by-name -a PXD004683 -f 20150820_Haura-Pilot-TMT2-bRPLC06-2.raw -o ~/pride_data/ -p aspera
  • Downloads now complete successfully without authentication prompts

Files Changed

  • pridepy/files/files.py: Changed key filename from aspera_tokenauth_id_rsa to asperaweb_id_dsa.openssh (2 locations)

PR Type

Bug fix


Description

  • Fixed Aspera key filename mismatch causing authentication failures

  • Updated key path from aspera_tokenauth_id_rsa to asperaweb_id_dsa.openssh

  • Resolves password/passphrase prompts during Aspera protocol downloads


Diagram Walkthrough

flowchart LR
  A["Incorrect key filename<br/>aspera_tokenauth_id_rsa"] -- "Fix mismatch" --> B["Correct key filename<br/>asperaweb_id_dsa.openssh"]
  B --> C["Aspera downloads<br/>work without prompts"]
Loading

File Walkthrough

Relevant files
Bug fix
files.py
Correct Aspera key filename in download method                     

pridepy/files/files.py

  • Updated Aspera key filename in download_files_from_aspera method
  • Changed from aspera_tokenauth_id_rsa to asperaweb_id_dsa.openssh
  • Fixes authentication failures when downloading files via Aspera
    protocol
+1/-1     

Summary by CodeRabbit

  • Bug Fixes
    • Updated authentication configuration for Aspera file downloads to use the correct key credentials.

The code was looking for 'aspera_tokenauth_id_rsa' but the actual
key file is named 'asperaweb_id_dsa.openssh'. This caused authentication
failures when downloading files via Aspera protocol.

Fixes authentication prompt issue when using aspera protocol.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

The Aspera private key file path reference was updated from aspera_tokenauth_id_rsa to asperaweb_id_dsa.openssh in the file download function. No logic or error handling modifications were made.

Changes

Cohort / File(s) Summary
Aspera key path update
pridepy/files/files.py
Updated hardcoded Aspera private key file path in download_files_from_aspera function from "aspera/key/aspera_tokenauth_id_rsa" to "aspera/key/asperaweb_id_dsa.openssh"

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A key path changed, simple and neat,
From tokenauth to web, now complete!
No logic altered, just credentials evolved,
Another small puzzle for Aspera solved. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Fix Aspera key filename to match actual key file" accurately and specifically describes the primary change in the pull request. The changeset exclusively updates the Aspera key filename from "aspera_tokenauth_id_rsa" to "asperaweb_id_dsa.openssh" in pridepy/files/files.py, which is precisely what the title communicates. The title is concise, clear, and provides sufficient context for a teammate reviewing the commit history to understand that this fixes a filename mismatch issue that was causing authentication failures. It meets all the criteria for a well-formed PR title.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21064b2 and a1a1621.

📒 Files selected for processing (1)
  • pridepy/files/files.py (1 hunks)
🔇 Additional comments (1)
pridepy/files/files.py (1)

286-288: Key file path change verified and approved.

Verification confirms:

  • No remaining references to old key filename aspera_tokenauth_id_rsa anywhere in the codebase
  • New filename asperaweb_id_dsa.openssh used only at the correct location (line 287)
  • Key file exists in repository at pridepy/aspera/key/asperaweb_id_dsa.openssh

The fix resolves the authentication failure issue and has been validated through PR testing.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use robust path handling for resources

Use the importlib.resources.as_file context manager to robustly access the
Aspera key file. This ensures it's available on the filesystem even when the
package is installed as a zip archive, preventing potential failures.

pridepy/files/files.py [286-323]

-key_full_path = importlib.resources.files("pridepy").joinpath(
+key_file_ref = importlib.resources.files("pridepy").joinpath(
     "aspera/key/asperaweb_id_dsa.openssh"
 )
-key_path = os.path.abspath(key_full_path)
-for file in file_list_json:
-    if file["publicFileLocations"][0]["name"] == "Aspera Protocol":
-        download_url = file["publicFileLocations"][0]["value"]
-    else:
-        download_url = file["publicFileLocations"][1]["value"]
-    file_name = os.path.basename(urlparse(download_url).path)
-    new_file_path = Files.get_output_file_name(download_url, file, output_folder)
-    if skip_if_downloaded_already and os.path.exists(new_file_path):
-        logging.info(f"Skipping download of {file_name} as it already exists.")
-        continue
-    try:
-        command = [
-            ascp_path,
-            "-QT",
-            "-l",
-            maximum_bandwidth,
-            "-i",
-            key_path,
-            f"era-fasp@{download_url}",
-            new_file_path,
-        ]
-        logging.info(f"Downloading {file_name} using Aspera")
-        subprocess.run(command, check=True)
-    except Exception as e:
-        logging.error(f"Aspera download failed for {new_file_path}: {str(e)}")
+with importlib.resources.as_file(key_file_ref) as key_path:
+    for file in file_list_json:
+        if file["publicFileLocations"][0]["name"] == "Aspera Protocol":
+            download_url = file["publicFileLocations"][0]["value"]
+        else:
+            download_url = file["publicFileLocations"][1]["value"]
+        file_name = os.path.basename(urlparse(download_url).path)
+        new_file_path = Files.get_output_file_name(download_url, file, output_folder)
+        if skip_if_downloaded_already and os.path.exists(new_file_path):
+            logging.info(f"Skipping download of {file_name} as it already exists.")
+            continue
+        try:
+            command = [
+                ascp_path,
+                "-QT",
+                "-l",
+                maximum_bandwidth,
+                "-i",
+                str(key_path),
+                f"era-fasp@{download_url}",
+                new_file_path,
+            ]
+            logging.info(f"Downloading {file_name} using Aspera")
+            subprocess.run(command, check=True)
+        except Exception as e:
+            logging.error(f"Aspera download failed for {new_file_path}: {str(e)}")
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential runtime error when accessing package resources in certain environments (e.g., zipped packages) and proposes a robust solution using importlib.resources.as_file, which significantly improves the code's reliability.

Medium
  • More

@ypriverol ypriverol requested a review from Copilot October 23, 2025 18:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical bug in the Aspera authentication configuration where the code referenced an incorrect key filename, causing authentication failures during file downloads.

Key Changes:

  • Corrected the Aspera SSH key filename from aspera_tokenauth_id_rsa to asperaweb_id_dsa.openssh to match the actual key file in the repository
  • This resolves password/passphrase prompts that prevented successful Aspera protocol downloads

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ypriverol ypriverol self-requested a review October 23, 2025 18:54
@ypriverol ypriverol merged commit 70cd964 into PRIDE-Archive:master Oct 23, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants