Refactor/transport boundary - #250
Open
chinmay1819 wants to merge 8 commits into
Open
Conversation
Registry combined OCI logic, request preparation and HTTP execution. This separates out the execution step, so the business logic deals with urls, headers and responses rather than with sessions and TLS. - add oras.transport.Transport, which owns the requests session, TLS verify and cookie policy, and is now the only place a request is sent. do_request keeps the authentication retry policy, and no longer repeats the same session call four times. - Registry.session and Registry._tls_verify become properties over the transport, so existing access and the sharing of the session with the auth backend are unchanged. - add get_manifest_content and upload_manifest_content, for manifests that have to move as exact bytes: re-serializing a parsed manifest can change its digest when it was written by another tool. - Layout uses those two methods instead of provider.do_request and hand built manifest urls, so its OCI traversal no longer depends on how requests are sent. - de-duplicate the blob upload session start shared by put_upload and chunked_upload. No public API is removed or renamed. Existing behaviour is unchanged, including urls, headers, status code handling, retries and error messages.
Auth backends held their own requests session and applied TLS verification themselves, so registry traffic and token requests were executed in two different places. That leaves the HTTP boundary incomplete: anything that changes how a request is sent has to be changed in both. Give the auth backend a transport instead, and let the provider hand over its own, so a token request and a manifest request share one session and one connection pool. Transport.request grows a params argument, which is what a token request needs and what previously forced auth to go around the transport. Also allow a transport to be passed to Registry, defaulting to the one built from tls_verify as before, and add an optional close() for callers that want pooled connections released promptly. Registry keeps the authentication challenge and the retry policy; the transport only carries out a request. session and _tls_verify are still readable and writable on both the provider and the auth backend, so existing access is unaffected.
Registry mixed two things: deciding what a registry operation should do, and carrying it out. Deciding is the larger part - building urls, preparing a layer from a file, merging annotations, choosing the manifest config, resolving where a pulled layer is written, reading what a response means - and none of it depends on how a request is sent. Move that half into a RegistryBase that Registry now derives from, and have push and pull drive it through _iter_push_layers, _apply_manifest_annotations, _prepare_manifest_config and _iter_pull_targets rather than inlining the same steps. push loses about seventy lines to methods that describe what they decide. response_reason is added next to the transport because requests and httpx name the status line reason differently, and shared code should not have to know which client produced a response. Behaviour is unchanged: the same requests are sent with the same headers, the same errors are raised with the same messages, and the full suite gives the same result as before the move.
Adds AsyncOrasClient alongside OrasClient, for callers that want to talk to a registry without blocking. The synchronous client is untouched and needs nothing new installed. The two are separate classes rather than one class with two modes, so whether a call is awaited is decided when the client is created and is visible at the call site. There is no event loop detection and no value that is sometimes a result and sometimes a coroutine. AsyncRegistry derives from the same RegistryBase as Registry, so it inherits the registry decisions rather than restating them; what it adds is the orchestration that has to await. AsyncTransport holds one httpx.AsyncClient for its lifetime, so connections are pooled, and the client is closed with `async with` or aclose(). Authentication answers a challenge asynchronously: the parsing, the choice of anonymous or basic, and the token cache are shared with the synchronous backend, and only the token request itself is awaited. ECR is the exception, because the AWS SDK is synchronous and has no async form without another dependency; that one call is run in a worker thread, once per realm. Downloads stream through httpx and are written in chunks, and uploads send an iterator over the file, so a large artifact is not held in memory in either direction. httpx is an extra rather than a requirement, so installing oras still pulls in only jsonschema and requests. Asking for the async client without it raises with a message naming the extra.
chinmay1819
marked this pull request as draft
August 18, 2026 08:18
chinmay1819
marked this pull request as ready for review
August 18, 2026 08:28
Author
|
@vsoch Please let me know if any changes needed. |
Contributor
|
I am out until September and then on travel, so your best bit is to ping someone in the oras slack to review. Thanks! |
Author
|
@jdolitsky could you please review this pr and let me know if any changes are required ? |
Author
|
Hi @vsoch , sorry to disturb you on your holidays, but I have tried with multiple people to review the pr on slack channel but as all of them are pre-occupied with work, no one is ready to review & approve the pr. Can you please take a look into this :) |
TerryHowe
reviewed
Aug 31, 2026
TerryHowe
left a comment
TerryHowe
left a comment
Member
There was a problem hiding this comment.
Critical Issues
- [oras/provider_async.py:350] The async blob upload body is a one-shot generator and is silently re-sent empty on the auth retry. put_upload passes data=iter_file(blob, ...) into do_request, which re-sends the same data object on a 401 challenge, on the 403 refresh, and on every retry_async attempt. An async generator can only be consumed once, so the second send transmits zero bytes while still declaring Content-Length: layer["size"] and the digest= query param. I reproduced it with a fake transport that 401s the first PUT:
PUT .../uploads/1?digest=sha256:b94d27... body= b'hello world'
PUT .../uploads/1?digest=sha256:b94d27... body= b'' <-- auth retry
- A real registry will either hang on the unfulfilled Content-Length or reject on digest mismatch. This is a regression vs. sync, not a mirrored bug: oras/provider.py put_upload uses data=fd.read() (bytes), which is re-sendable. Fix by materializing the body, or by re-creating the iterator per attempt (e.g. pass a factory and call it inside the transport).
- [oras/provider_async.py:294] download_blob bypasses do_request, so it has no authentication-challenge handling and no retry. It calls self.transport.stream(...) directly and only attaches a header when isinstance(self.auth, TokenAuth) and self.auth.token is not None. Consequences: with BasicAuth no Authorization header is ever sent; with EcrAuth/TokenAuth before a token is cached, the response is a 401 that raise_for_status() turns into an exception instead of a challenge to answer; and a token scoped narrower than the blob endpoint can never be refreshed. Sync goes download_blob → get_blob → do_request, which handles all three. This is invisible in the common pull() path only because get_manifest happens to warm the token first.
Important Issues
- [oras/decorator.py:118] retry_async loses the sync fast-fail on TLS errors. Sync retry has except (requests.exceptions.SSLError, ImportError): raise; the async version keeps only ImportError. httpx surfaces certificate failures as httpx.ConnectError (a plain Exception), so a bad cert or wrong hostname now retries 5 times with 2+3**n backoff — roughly 48 s of sleeping — before the user sees the error. Add the httpx connect/SSL classes to the re-raise tuple (lazily, to keep httpx optional).
- [oras/transport_async.py:130] stream() never calls _forget_cookies(). The module docstring and _forget_cookies exist specifically for the Harbor CSRF case, but the streaming path — blob downloads, i.e. most of the traffic volume — leaves whatever the registry set sitting in the httpx jar for subsequent requests. Related: because _forget_cookies runs after request() returns, cookies are still honored across redirects within a single request, whereas the sync DefaultCookiePolicy(allowed_domains=[]) refuses them outright. Worth closing both gaps.
- DCO is failing and no test workflow has run. None of the five commits carry a Signed-off-by trailer, and the head SHA's only check is DCO: action_required — the auth-tests workflow has never executed on this branch. That means every async integration test (test_async_push_pull_*, test_async_chunked_upload, test_async_index_is_pushed_and_read_back) is currently unverified against a real registry, which is exactly where the two critical issues above would surface. Rebase with git rebase --signoff (and squash the Merge branch 'oras-project:main' commit) so CI can run.
Member
|
DCO failure as well, might want to squash the commits and signoff the one commit. |
Author
|
Thanks for reviewing @TerryHowe . I will look into the mentioned issues :) |
A blob upload passed its body to do_request as an iterator over the file. That iterator is created once at the call site, but the request can be sent more than once: to answer a 401 challenge, to refresh a token after a 403, or because the retry decorator tried again. An iterator is spent after the first read, so every later attempt sent an empty body while Content-Length and the digest query parameter still described the real content. Let a body be given as a callable that produces a fresh one, and resolve it per attempt in both providers. The resolution lives on RegistryBase, since deciding what to send is a registry decision rather than transport work, and both execution models need it. Bodies that can be read repeatedly, which is every other caller, are passed through untouched. The upload keeps streaming from disk; only the moment the iterator is created has changed. Covered by tests that read the body on each send, for the authentication path and for the retry path, and that fail without this change.
The previous fix produced a fresh body for each attempt do_request made, which covered the authentication challenge and the retry decorator. It did not cover redirects. Registries backed by object storage redirect a blob upload, and httpx follows that redirect inside a single call, below the point where the body had already been resolved. httpx cannot replay a body it has streamed, so the upload raised StreamConsumed after sending the redirected request with an empty body and the original Content-Length. It failed the same way on every retry, so uploads to those registries could not succeed. Move the resolution down to the transports, which are what actually send, and have the async transport follow 307 and 308 itself when the body can be produced again, asking for a fresh one each hop. Only those two are followed, since they are the redirects that keep the method and the body. Bodies that can be read repeatedly, which is every other caller, are still left to httpx. The synchronous provider was never affected: its upload passes bytes, which requests can re-send. It shares the same helper so the two behave alike. Covered by a test that redirects a real streamed upload through a real server, because the failure lives inside httpx's redirect handling, below where a fake transport would sit. It raises StreamConsumed without this change.
download_blob opened a stream on the transport directly, so it was the one registry call that never went past do_request. It attached an Authorization header only when the backend was TokenAuth and a token happened to be cached already, and it treated whatever came back as final. That left three ways to fail. Basic auth never sent credentials, because they are added when a challenge arrives and nothing here read a challenge. A token backend with nothing cached got a 401, which raise_for_status turned into an exception rather than something to answer. And a token scoped for the manifest but not the blob endpoint could never be refreshed. It stayed hidden in the usual pull, where get_manifest warms the token first, and would have surfaced on a download on its own or against a registry that scopes tokens per endpoint. do_request cannot serve this path: it returns a response whose body has been read, which is what a download has to avoid. Add stream_request as its streaming counterpart, answering a challenge the same way - once, then once more with a refreshed token if the retry is still refused - and yielding a response whose body is still on the wire. Reopen the stream rather than replay it, since the challenge is in the headers and the body is never needed. Downloads also had no retry, having skipped the decorator, so a failed connection now gets another attempt with the same backoff the decorator uses. The file is opened inside the attempt so a retry starts clean. A status the registry meant, a 404 say, is not retried. The synchronous path already did all of this through get_blob and do_request.
TerryHowe
approved these changes
Sep 4, 2026
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.
Summary
This PR addresses #242 by introducing the architectural foundation required for native asynchronous support in ORAS-py.
The implementation separates Registry/OCI behavior from HTTP transport concerns, allowing synchronous and asynchronous execution paths to share the same high-level logic while using transport implementations appropriate to each execution model.
The async API is designed as an explicit API rather than making existing synchronous methods dynamically behave as either synchronous or asynchronous.
Issue
Closes #242
Reviewer
@vsoch — Vanessa Sochat, please review the architectural direction and implementation.
Architectural Strategy
The main design goal is to avoid maintaining two independent implementations of the Registry logic.
Instead of duplicating the existing
Registryimplementation into anAsyncRegistry, the architecture separates:The intended architecture is: