linkd

[xla]

First iterative implementation of RFC 0696 focusing on driving the core
peer protocol and establishing the harness for configuration of a running node.
Rite of passage for it was the replacement of the ephemeral peer in the e2e
networkss, as that required a well behaved peers with a set of knobs exposed.

Structurally the majority implementation lives in a library crate linkd-lib -
probably the most horrendous crate name in this repo to date - with a newly
created workspace under bins and a linkd crate in wrapping it lightly in a
main. That strategy allows for a checked-in Cargo.lock for binaries
associated with link project. Beyond that the code is rather Mario-esque as it
is primarily plumbing of code that either existed in other crates backing
binaries or newish code doing the same thing for core pieces.

There are still large pieces missing, which a tracked in #722 and will be
piled on-top as patches to keep this already big delta focused. The declared
initial goal is to get linkd into a state where it can run as bootstrap/seed
node.

[FintanH]

Just some small things so far, but it's looking like it's coming together :)

[kim]

IIUC, setting this to true will render rad_home a no-op. Perhaps fold into one argument?

[FintanH]

Knew I'd bring you around to nix sooner or later

[xla]

Not your kind of nix, boy!

[FintanH]

Are these all for the follow-up patches of #722?

[xla]

With our rapid offboarding of this walled garden I'm uncertain where to track follow-up work. Will go over the TODOs and see if they are out-of-sync with the #722.

[FintanH]

If the above says required_if("metrics-provider", "graphite"), wouldn't it make sense that this should be Some(MetricsProvider::Graphite) since the address is provided below? 🤔

[xla]

You are right, I haven't found a way to enforce a heuristic a la: only expect X when Y is set.

[FintanH]

Ya, I meant the way one would usually do this is by packing it as an enum variant, right?

enum Provider {
   Graphite { address: String }
}

but that doesn't fly if we want flattened args, so I guess we're between a rock and a hard place :) I wonder would it make sense to have the graphite_addr as Option as well?

[xla]

Might be I'm holding the Structops wrong but that would conflict with the need to default to the local graphite instance if graphite is set as a metric provider.

[FintanH]

My thinking is that if metrics-provider is set to "graphite" then you'll end up with Some(MetricsProvider::Graphite) and then the address will be Some("localhost:8080") (or whatever the port is). And on the flip-side, if metrics-provider is None then graphite_addr is also, rightfully, None.

That being said this is not a big deal, and should not be a blocker :) Just musing about what's the best way to represent the data

[xla]

I don't follow how you'd express that with the structopt primitives.

[FintanH]

My suggestion is:

#[derive(Debug, Eq, PartialEq, StructOpt)]
pub struct MetricsArgs {
    /// Provider for metrics collection.
    #[structopt(long = "metrics-provider", name = "metrics-provider")]
    pub provider: Option<MetricsProvider>,

    /// Address of the graphite collector to send stats to.
    #[structopt(
        long,
        required_if("metrics-provider", "graphite")
    )]
    pub graphite_addr: Option<String>,
}

/* then the conversion here */
        let metrics = match args.metrics.provider {
            Some(args::MetricsProvider::Graphite) => Some(Metrics::Graphite(
                args.metrics
                    .graphite_addr
                    .unwrap_or("localhost:2003")
                    .to_socket_addrs()?
                    .next()
                    .unwrap(),
            )),
            None => None,
        };

Which is a minor improvement if any. It's just unfortunate that we can't express this better in structopt.

[FintanH]

Nice work =] Looking forward to the new age of daemons

[kim]

I would, however, appreciate a slightly more condensed commit history.

[xla]

I would, however, appreciate a slightly more condensed commit history.

I always squash before merge but leave that to the latest point to account for commits addressing feedback.