Fix slow downloads and read-only config exports - #307
Merged
Conversation
with ftp downloads repeated cases of multi-hour downloads of files that could be https downloaded in minutes were occurring. This is likely due to ftp throttling. By using https this change aims to improve reliability and speed.
shutil.copytree defaults to copy2 which preserves source permissions. Package resources in the Nix store are read-only (0444), so exported config files were uneditable without a manual chmod, complicating automated workflows. Pass copy_function=shutil.copyfile to copy content only, allowing the destination permissions to be set by the user's umask (typically 0644).
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces FTP-based bulk file downloads with streaming HTTPS using httpx to fix slow transfers, and updates demo-config export logic to use a content-only copy so configs are writable even when installed from a read-only Nix store, adding httpx as an explicit dependency. Sequence diagram for HTTPS-based file download in _copy_to_localsequenceDiagram
actor User
participant CLI as cli_download_command
participant Downloader as _copy_to_local
participant HTTPClient as httpx_client
participant EBIServer as ftp_ebi_ac_uk_https
participant FS as local_filesystem
User->>CLI: run_eti_download
CLI->>Downloader: _copy_to_local(src, dest, host)
Downloader->>FS: check_dest_exists(dest)
FS-->>Downloader: dest_missing
Downloader->>HTTPClient: stream_get(url=https://host/src, follow_redirects=True, timeout=300.0)
HTTPClient->>EBIServer: HTTPS_GET_stream(url)
EBIServer-->>HTTPClient: streaming_response
HTTPClient-->>Downloader: response_handle
Downloader->>HTTPClient: response_raise_for_status()
loop for_each_chunk
Downloader->>HTTPClient: response_iter_bytes(chunk_size=1048576)
HTTPClient-->>Downloader: chunk_bytes
Downloader->>FS: atomic_write_chunk(dest, chunk_bytes)
end
Downloader-->>CLI: return_dest_path(dest)
CLI-->>User: download_complete
Flow diagram for _copy_to_local HTTPS streaming logicflowchart TD
A["_copy_to_local called"] --> B["Check if dest exists"]
B -->|exists| C["Return existing dest path"]
B -->|does_not_exist| D["Build HTTPS URL from host and src"]
D --> E["Open httpx streaming GET with follow_redirects=True timeout=300.0"]
E --> F["response.raise_for_status()"]
F --> G["Open atomic_write(dest, mode='wb')"]
G --> H{"More chunks from response.iter_bytes(chunk_size=1048576)?"}
H -->|yes| I["Read next chunk bytes"]
I --> J["Write chunk to outfile"]
J --> H
H -->|no| K["Close response and outfile"]
K --> L["Return dest path"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
GavinHuttley
requested changes
Mar 12, 2026
GavinHuttley
left a comment
GavinHuttley
left a comment
There was a problem hiding this comment.
I understand the motivation and am happy to add this in. Moving to https opens another improvement opportunity here if you are interested and have the time. The approach is to use threading and the ability to download a large file in chunks, requesting a range of bytes and writing those into the correct location. Pasting some demo code from another project below
def _fetch_range(
url: str,
dest: pathlib.Path,
start: int,
end: int,
progress: rich.progress.Progress,
task: rich.progress.TaskID,
) -> None:
"""Fetch a byte range of *url* and write it into *dest* at the correct offset."""
req = urllib.request.Request(url) # noqa: S310
req.add_header("Range", f"bytes={start}-{end}")
resp = urllib.request.urlopen(req) # noqa: S310
with open(dest, "r+b") as f:
f.seek(start)
while data := resp.read(1024 * 64):
f.write(data)
progress.update(task, advance=len(data))
def _download_parallel(
name: str, url: str, dest: pathlib.Path, total: int, connections: int
) -> None:
"""Download a URL using parallel byte-range requests."""
# pre-allocate the output file
with open(dest, "wb") as f:
f.truncate(total)
chunk_size = total // connections
ranges = []
for i in range(connections):
start = i * chunk_size
end = total - 1 if i == connections - 1 else (i + 1) * chunk_size - 1
ranges.append((start, end))
with _make_progress(name) as progress:
task = progress.add_task("download", total=total)
with ThreadPoolExecutor(max_workers=connections) as pool:
futures = [
pool.submit(_fetch_range, url, dest, s, e, progress, task)
for s, e in ranges
]
for future in as_completed(futures):
future.result()
Collaborator
Pull Request Test Coverage Report for Build 23123677479Details
💛 - Coveralls |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fix slow downloads and read-only config exports
Summary
Two independent bug fixes, each a small targeted change.
Fix 1: Use HTTPS instead of FTP for file downloads
File:
src/ensembl_tui/_ftp_download.pyProblem
File downloads via
eti downloadwere extremely slow compared todownloading the same files directly via a browser. A genome FASTA file
that downloaded in ~4 minutes via HTTPS took hours via
eti download.The root cause is that
_copy_to_localuses Python'sftplib.FTP—raw FTP protocol — while browsers use HTTPS. EBI exposes an HTTPS
gateway at the same hostname (
ftp.ebi.ac.uk) serving identicalcontent. Switching to HTTPS resolved the performance issue.
The
listdirfunction (used for directory listings) anddownload_species_table(used for the species TSV) retain FTP — theseare small, fast operations where the slowness was not observed.
Note that
_download.pyalready uses HTTPS for tree downloads viacogent3.load_tree, establishing the pattern.Change
Replace the
ftplib.retrbinarytransfer in_copy_to_localwith anhttpx.streamGET request against the HTTPS endpoint.httpxisalready a transitive dependency of
ensembl_tui.Fix 2: demo-config exports read-only files when installed via Nix
File:
src/ensembl_tui/cli.pyProblem
demo_configcallsshutil.copytreeto copy package resource files(including
sample.cfg) out of the installed package into the user'schosen output directory.
copytreedefaults toshutil.copy2as itscopy function, which preserves source file permissions including the
read-only
0444mode set on all files in the Nix store. The exportedconfig files are then uneditable without a manual
chmod.Requiring a manual
chmodafter everydemo-configinvocationcomplicates automated setup workflows and is likely to produce
confusing errors for users who are unaware of the Nix store's
read-only semantics.
Change
Pass
copy_function=shutil.copyfileto copy content only, withoutpreserving permissions. The destination files receive permissions set
by the user's umask (typically
0644).shutil.copyfilematches thecopy_functionsignature and is thestandard library's content-only copy primitive.
Summary by Sourcery
Switch downloads to use HTTPS streaming and adjust demo-config exports to produce writable files, while adding the HTTP client dependency.
Bug Fixes:
Build: