SY-4130: Add Python client support for server-side import and export#2324
SY-4130: Add Python client support for server-side import and export#2324pjdotson wants to merge 35 commits into
Conversation
…ide-import-and-export
…ide-import-and-export
…ide-import-and-export
…ide-import-and-export
…ide-import-and-export
…ide-import-and-export
…nt-support-for-server-side-import-and-export
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc #2324 +/- ##
==========================================
- Coverage 66.53% 66.18% -0.36%
==========================================
Files 2602 2444 -158
Lines 115123 113048 -2075
Branches 8610 8678 +68
==========================================
- Hits 76601 74821 -1780
+ Misses 32316 32120 -196
+ Partials 6206 6107 -99
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…nt-support-for-server-side-import-and-export # Conflicts: # freighter/integration/go.mod # freighter/integration/go.sum
…nt-support-for-server-side-import-and-export
…ide-import-and-export
emilbon99
left a comment
There was a problem hiding this comment.
- Concerns about the protocol class additions. Why can't we just use
UnaryClient? The download and upload clients seem like relatively minor variations. - What happens when we do import/export of compound data structures i.e. entire system configurations? Do we care about supporting that in the imex client?
| codec) need only satisfy Codec. | ||
| """ | ||
|
|
||
| def file_extension(self) -> str: |
There was a problem hiding this comment.
Should this just be called extension?
| from freighter.transport import RQ, FilePath, Transport | ||
|
|
||
|
|
||
| class DownloadClient(Transport, Protocol): |
There was a problem hiding this comment.
I don't totally understand why these need a different protocol than Unary. Why isn't the DownloadClient something like a DownloadTransport that is a concrete implementation of Unary whose request type accepts a destination as its file type? Seems like we should be trying to avoid adding different transport types to freighter if possible.
…nt-support-for-server-side-import-and-export
…nt-support-for-server-side-import-and-export
…ide-import-and-export
Issue Pull Request
Linear Issue
SY-4130
Description
Add Python client support for server-side import and export.
Basic Readiness
Greptile Summary
This PR adds Python client support for server-side import and export (
imex) via three new transport modes: in-memory typed send, file-path-based streaming upload, and streaming download to a destination path.freighter/pygainsUploadClientandDownloadClientprotocols;HTTPClientis refactored to accept an encoder and a list of decoders for content negotiation, replacing the single-codec approach.synnax.imexmodule wraps these transports, andcore/pkg/api/http/http.gorenames the ImEx routes from/api/v1/{import,export}to/api/v1/imex/{import,export}to match.MsgPackCodecis renamedMessagePackCodec, and both codecs gain afile_extension()method used to infer wire format from a file path.Confidence Score: 5/5
Safe to merge; the new imex transport layer and server-side route rename are self-contained and well-tested.
The core transport refactor in HTTPClient is clean and backed by thorough upload/download tests. The imex client, Envelope type, and Go route rename all look correct. The only open items are known deferred clean-ups already acknowledged by the team, and a couple of minor documentation inaccuracies.
No files require special attention.
Important Files Changed
id(already noted in thread). Logic is straightforward.Sequence Diagram
sequenceDiagram participant User participant ImexClient participant HTTPClient participant Server Note over User,Server: import_(Envelope) — in-memory typed send User->>ImexClient: import_(envelope) ImexClient->>HTTPClient: send("/imex/import", envelope, _ImportResponse) HTTPClient->>Server: POST /api/v1/imex/import Content-Type: application/json Server-->>HTTPClient: "200 {key: uuid}" HTTPClient-->>ImexClient: (_ImportResponse, None) ImexClient-->>User: key: str Note over User,Server: import_(FilePath) — streaming file upload User->>ImexClient: import_(path) ImexClient->>HTTPClient: upload("/imex/import", path, _ImportResponse) HTTPClient->>Server: POST /api/v1/imex/import Content-Type: inferred from extension Server-->>HTTPClient: "200 {key: uuid}" HTTPClient-->>ImexClient: (_ImportResponse, None) ImexClient-->>User: key: str Note over User,Server: export(id) — in-memory typed response User->>ImexClient: export(id) ImexClient->>HTTPClient: send("/imex/export", id, Envelope) HTTPClient->>Server: POST /api/v1/imex/export Accept: application/json Server-->>HTTPClient: 200 Envelope JSON HTTPClient-->>ImexClient: (Envelope, None) ImexClient-->>User: Envelope Note over User,Server: export(id, dest=path) — streaming download User->>ImexClient: "export(id, dest=path)" ImexClient->>HTTPClient: download("/imex/export", id, dest) HTTPClient->>Server: POST /api/v1/imex/export Accept: inferred from dest extension Server-->>HTTPClient: 200 chunked body HTTPClient->>HTTPClient: stream chunks to dest file HTTPClient-->>ImexClient: None ImexClient-->>User: NoneComments Outside Diff (1)
freighter/py/freighter/http.py, line 250-262 (link)_decode_errordelegates to_resolve_decoder, which returns a genericValueErrorwhen the error response carries an unregistered or absentContent-Type. The real error bytes are embedded in the message but not decoded, so callers see "no decoder" rather than the server's actual message. Any proxy or gateway returningtext/htmlon a 4xx/5xx will lose its error message entirely.Reviews (12): Last reviewed commit: "Merge branch 'rc' into sy-4130-add-pytho..." | Re-trigger Greptile