feat: init custom activation #18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/custom-activation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -83,6 +83,7 @@
devShell = pkgs.mkShell {
RUST_SRC_PATH = "${pkgs.rustPlatform.rustLibSrc}";
NIX_PATH = "nixpkgs=${pkgs.path}";
PROTOC = "${pkgs.protobuf}/bin/protoc";
getExe' pkgs.protobuf "protoc"
d6bad0d85f
to04dfa25d0b
04dfa25d0b
toc6720d7bc8
fc7964eb0a
tod9e41ab07d
21bf640c47
to3ca9b95f44
WIP: custom activationto feat: init custom activation3ca9b95f44
to747f87e4bd
@ -0,0 +72,4 @@
targets: Arc<Mutex<HashMap<Id, TH>>>,
}
impl<TH, AT, BT, CT> ActivationServiceImpl<TH, AT, BT, CT>
Tu veux pas juste prendre des Box au lieu de monomorphiser des async fn? Ça m'a l'air extrêmement fragile et compliqué
j'avais fait pas mal de recherches et de tests qui concluaient que ça ne pouvait pas marcher
mais j'ai refait une recherche pour être sûr aujoud'hui et j'ai vu qu'en utilisant https://docs.rs/futures/latest/futures/future/type.BoxFuture.html ça pourrait peut-être fonctioner...
Après des tests plus approfondis, je pense que ça ne fonctionnera pas
Si tu as une idée plus précise d'implémentation qui respectent toutes les conditions et qui soit pas plus compliqué que la solution actuelle, je veux bien l'implémenter
Par ailleurs je pense pas que la solution actuelle soit fragile (mais extrêmement compliqué, oui...)
Je disais "fragile" dans le sens où le jour où tu sors un objet chelou qui remplit pas une trait condition, tu te retrouves à debug l'enfer mais OK
J'avais compris ce sens, mais si tu remplis pas une trait condition, même si tu boxes la chose, ça atterit sur le même résultat.
Normalement j'ai fait en sorte que les traits doivent couvrir le maximum de cas (sauf peut-être la condition 'static)
@ -0,0 +8,4 @@
RequestHeader header = 1;
string toplevel = 2;
ApplyType apply_type = 3;
bool reboot = 4;
should be an enum so that reboot can take on multiple meanings: system reboot, kexec and leave space for more meanings later.
@ -0,0 +6,4 @@
message BuildRequest {
RequestHeader header = 1;
string derivation = 2;
bool remote = 3;
What does this mean?
remote building, i'll rename it
remote_building
@ -0,0 +36,4 @@
/// The gRPC connection to the activation program
grpc_client: ActivationClient<Channel>,
session_id: String,
A comment?
@ -0,0 +70,4 @@
let mut grpc_client = None;
for _ in 0..5 {
This should use exponential backoff and the number of retries and spacing should be configurable I would say, but you can punt a TODO for it.
Yep, but this is here for now only to let
activation_program
the time to boot (which, in my tests, and the several programs I wrote, is always less than 5 seconds)@ -0,0 +106,4 @@
header: Some(RequestHeader {
session_id: self.session_id.clone(),
}),
derivation: derivation.as_path().to_str().unwrap().to_string(),
Won't this lead to
"x"
instead ofx
in the string format?to_string()
seems always the wrong method to call.I don't understand your point.
If you mean that
to_string()
while add quote around the string, it is wrong.str.to_string()
==str.to_owned()
(including performance)I find
to_string()
clearer that we will actually convert it to aString
@ -0,0 +195,4 @@
}),
store_path: closure
.as_path()
.as_os_str()
Big mehhhhh, use
String::from_utf8_lossy
and prepare for crashes when you encounter a non-UTF8 store path.Aren't store paths always UTF-8 in nix?
my sweet summer child…
@ -0,0 +1,109 @@
#![allow(clippy::pedantic)]
tonic::include_proto!("colmena.activation");
pub trait HeadedResponse {
Comment on what this is.
@ -0,0 +63,4 @@
}
fn from_header(header: ResponseHeader) -> Self {
let new_store_path = if header.msg.starts_with("/nix/store") {
Why can we make this assumption?
Messages can be log of the building too, so not every message is a nix store path.
@ -0,0 +250,4 @@
let session_id = Id(id);
let mut lock = self.targets.lock().await;
let mut targets =
no?@ -0,0 +162,4 @@
}
};
let lock = self.targets.lock().await;
let targets =
no?@ -0,0 +257,4 @@
session_id: session_id
.0
.iter()
.fold(String::new(), |s, i| format!("{s}{i:02x}")),
Can you explain the transformation you want to do here?
@ -0,0 +94,4 @@
.lines();
loop {
for stderr_line in stderr.by_ref() {
Mixing stderr and stdout in the same log seems a bit of a shame, no?
I think we can because, the processing is a bit different:
OK, wfm
@ -0,0 +20,4 @@
},
}
impl TryFrom<Url> for TargetHost {
Warning about URI encoding, space is
%20%
, your display formatter does not reintroduce this?Display formatter has only one purpose for now: printing to stderr for debugging, so it is not important (and
%20%
will flood the stderr with useless and unreadable information)@ -0,0 +10,4 @@
import "copy_closure_request.proto";
import "copy_closure_response.proto";
service Activation {
What's the overall story for versioning this protocol?
More explanations are required at this point. This seems mostly good to me as a first step.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.