-
Notifications
You must be signed in to change notification settings - Fork 42
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.
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
factor out export "protocol" into common package #1855
Comments
Agreed that we need a better way to share across components but if this is just one file I'd probably look for an easier way to duplicate stuff, like symlinks vs the overhead of a shared package. |
On Wed, Feb 21, 2024 at 03:03:29PM -0800, Kunal Mehta wrote:
Agreed that we need a better way to share across components but if
this is just one file I'd probably look for an easier way to duplicate
stuff, like symlinks vs the overhead of a shared package.
I leapt for the "common package" option because intuitively there's an
implicit versioned protocol here. But I think you're right
that it may be both adequate and simpler to regard it just as shared
code.
|
I think you're absolutely right on this, and we have the same pattern in proxy v2 that's just less obvious because it's in two different languages: /// Serialization format for non-streamed requests
#[derive(Serialize, Debug)]
struct OutgoingResponse {
status: u16,
headers: HashMap<String, String>,
body: String,
} @dataclass(frozen=True)
class JSONResponse:
data: dict
status: int
headers: Dict[str, str] (not identical, but pretty close)
To set up the shared package we'd:
If we think making it easier to share code will result in more code being shared then setting up the infrastructure for it makes sense, otherwise if it's just one or two files I think just having a test to keep them in sync or symlinks is simpler. In Rust we'd get to skip step 3 since all the dependencies are automatically compiled in and I'd definitely want to see us share e.g. serde structs to have nicer type safety on both sides. |
|
securedrop-client/client/securedrop_client/export_status.py
Lines 9 to 14 in 0555520
This suggests to me that we need a package
securedrop-export-common
, on which bothsecuredrop-client
andsecuredrop-export
depend.Originally posted by @cfm in #1777 (comment)
The text was updated successfully, but these errors were encountered: