Skip to content

🐛 use httpx to download output ports #7824

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 5, 2025
Merged

🐛 use httpx to download output ports #7824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f57c32
use aiohttp to download output ports
Jun 5, 2025
3d75c03
removed unused session parameter
Jun 5, 2025
d9af07c
Merge remote-tracking branch 'upstream/master' into pr-osparc-refacto…
Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/simcore-sdk/requirements/_base.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
aiocache
aiofiles
aiohttp
httpx
packaging
pint
sqlalchemy[asyncio]
pydantic[email]
sqlalchemy[asyncio]
tenacity
tqdm
25 changes: 25 additions & 0 deletions packages/simcore-sdk/requirements/_base.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ anyio==4.8.0
# via
# fast-depends
# faststream
# httpx
arrow==1.3.0
# via
# -r requirements/../../../packages/models-library/requirements/_base.in
Expand Down Expand Up @@ -72,6 +73,8 @@ certifi==2025.1.31
# -c requirements/../../../packages/settings-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/settings-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../requirements/constraints.txt
# httpcore
# httpx
# requests
charset-normalizer==3.4.1
# via requests
Expand Down Expand Up @@ -109,10 +112,32 @@ greenlet==3.1.1
# via sqlalchemy
grpcio==1.70.0
# via opentelemetry-exporter-otlp-proto-grpc
h11==0.16.0
# via httpcore
httpcore==1.0.9
# via httpx
httpx==0.28.1
# via
# -c requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/models-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/models-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/postgres-database/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/postgres-database/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../packages/models-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../packages/models-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../packages/settings-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../packages/settings-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/service-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/settings-library/requirements/../../../packages/common-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../packages/settings-library/requirements/../../../requirements/constraints.txt
# -c requirements/../../../requirements/constraints.txt
# -r requirements/_base.in
idna==3.10
# via
# anyio
# email-validator
# httpx
# requests
# yarl
importlib-metadata==8.5.0
Expand Down
27 changes: 18 additions & 9 deletions packages/simcore-sdk/src/simcore_sdk/node_ports_common/file_io_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from typing import IO, Any, Final, Protocol, runtime_checkable

import aiofiles
import httpx
from aiohttp import (
ClientConnectionError,
ClientError,
ClientPayloadError,
ClientResponse,
ClientResponseError,
ClientSession,
Expand Down Expand Up @@ -39,6 +39,7 @@
from tqdm.contrib.logging import tqdm_logging_redirect
from yarl import URL

from ..config.http_clients import client_request_settings
from . import exceptions
from .constants import CHUNK_SIZE

Expand Down Expand Up @@ -148,13 +149,13 @@ async def __call__(self, log: str) -> None: ...

async def _file_chunk_writer(
file: Path,
response: ClientResponse,
response: httpx.Response,
pbar: tqdm,
io_log_redirect_cb: LogRedirectCB | None,
progress_bar: ProgressBarData,
):
async with aiofiles.open(file, "wb") as file_pointer:
while chunk := await response.content.read(CHUNK_SIZE):
async for chunk in response.aiter_bytes(CHUNK_SIZE):
await file_pointer.write(chunk)
if io_log_redirect_cb and pbar.update(len(chunk)):
with log_catch(_logger, reraise=False):
Expand All @@ -172,7 +173,6 @@ async def _file_chunk_writer(


async def download_link_to_file(
session: ClientSession,
url: URL,
file_path: Path,
*,
Expand All @@ -185,16 +185,25 @@ async def download_link_to_file(
reraise=True,
wait=wait_exponential(min=1, max=10),
stop=stop_after_attempt(num_retries),
retry=retry_if_exception_type(ClientConnectionError),
retry=retry_if_exception_type(httpx.TransportError),
before_sleep=before_sleep_log(_logger, logging.WARNING, exc_info=True),
after=after_log(_logger, log_level=logging.ERROR),
):
with attempt:
async with AsyncExitStack() as stack:
response = await stack.enter_async_context(session.get(url))
if response.status == status.HTTP_404_NOT_FOUND:
client = await stack.enter_async_context(
httpx.AsyncClient(
Copy link
Member

@sanderegg sanderegg Jun 5, 2025

Choose a reason for hiding this comment

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

creating a new client everytime is costly and wasting resources.
here you are changing the behavior. I don't want to stop this now, but please take care of that in a next PR, thank you.
Create an issue and let's fix this calmly.

timeout=httpx.Timeout(
client_request_settings.HTTP_CLIENT_REQUEST_TOTAL_TIMEOUT
)
)
)
response = await stack.enter_async_context(
client.stream("GET", f"{url}")
)
if response.status_code == status.HTTP_404_NOT_FOUND:
raise exceptions.InvalidDownloadLinkError(url)
if response.status > _VALID_HTTP_STATUS_CODES:
if response.status_code > _VALID_HTTP_STATUS_CODES:
raise exceptions.TransferError(url)
file_path.parent.mkdir(parents=True, exist_ok=True)
# SEE https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Length
Expand Down Expand Up @@ -231,7 +240,7 @@ async def download_link_to_file(
sub_progress,
)
_logger.debug("Download complete")
except ClientPayloadError as exc:
except httpx.HTTPError as exc:
raise exceptions.TransferError(url) from exc


Expand Down
19 changes: 8 additions & 11 deletions packages/simcore-sdk/src/simcore_sdk/node_ports_common/filemanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ async def download_path_from_s3(
return await download_file_from_link(
download_link,
local_path,
client_session=session,
io_log_redirect_cb=io_log_redirect_cb,
progress_bar=progress_bar,
)
Expand All @@ -229,7 +228,6 @@ async def download_file_from_link(
*,
io_log_redirect_cb: LogRedirectCB | None,
file_name: str | None = None,
client_session: ClientSession | None = None,
progress_bar: ProgressBarData,
) -> Path:
# a download link looks something like:
Expand All @@ -242,15 +240,14 @@ async def download_file_from_link(

if io_log_redirect_cb:
await io_log_redirect_cb(f"downloading {local_file_path}, please wait...")
async with ClientSessionContextManager(client_session) as session:
await download_link_to_file(
session,
download_link,
local_file_path,
num_retries=NodePortsSettings.create_from_envs().NODE_PORTS_IO_NUM_RETRY_ATTEMPTS,
io_log_redirect_cb=io_log_redirect_cb,
progress_bar=progress_bar,
)

await download_link_to_file(
download_link,
local_file_path,
num_retries=NodePortsSettings.create_from_envs().NODE_PORTS_IO_NUM_RETRY_ATTEMPTS,
io_log_redirect_cb=io_log_redirect_cb,
progress_bar=progress_bar,
)
if io_log_redirect_cb:
await io_log_redirect_cb(f"download of {local_file_path} complete.")
return local_file_path
Expand Down
Loading