Skip to content

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Oct 10, 2023

Motivation

To start, it is just a clone of the common protocol. But now that we have the separate protocol implementations, we can add versioning information without the versions of one protocol leaking into another.

Using the infrastructure from the previous commit, we don't have to duplicate code for shared behavior.

Motivation: No more perverse incentives. fe1f34f did some awkward things because the serialisers did not store the version. I don't want anyone making changes to be pushed towards keeping the serialization logic with the core data types just because it's easier or the alternative is tedious.

Context

More progress breaking out #6223 into separate PRs, with added tests.

The actual versioning of the Worker and Serve protocol serialisers (Common remains unversioned as the underlying mini-protocols are not versioned) will happen in subsequent commits / PRs.

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority store Issues and pull requests concerning the Nix store labels Oct 10, 2023
@Ericson2314 Ericson2314 changed the title Introduce a separate serve protocol Introduce separate serve protocol serialisers Oct 10, 2023
To start, it is just a clone of the common protocol. But now that we
have the separate protocol implementations, we can add versioning
information without the versions of one protocol leaking into another.

Using the infrastructure from the previous commit, we don't have to
duplicate code for shared behavior.

Motivation: No more perverse incentives. [0] did some awkward things
because the serialisers did not store the version. I don't want anyone
making changes to be pushed towards keeping the serialization logic with
the core data types just because it's easier or the alternative is
tedious.

The actual versioning of the Worker and Serve protocol serialisers
(Common remains unversioned as the underlying mini-protocols are not
versioned) will happen in subsequent commits / PRs.

[0]: fe1f34f
@Ericson2314 Ericson2314 changed the title Introduce separate serve protocol serialisers Introduce separate Serve protocol serialisers Oct 10, 2023
@Ericson2314
Copy link
Member Author

Ericson2314 commented Oct 13, 2023

Because the change itself is from #6223 which is approved modulo tests, and the test plan identical to #9099 which is approved and merged, I think this is ready.

@Ericson2314 Ericson2314 merged commit d070d8b into NixOS:master Oct 13, 2023
@Ericson2314 Ericson2314 deleted the serve-protocol branch October 13, 2023 14:51
tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
Introduce separate Serve protocol serialisers

(cherry picked from commit d070d8b)
Change-Id: Ibcc8639e8997bcd07f7a5318330a147bcadc411b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant