cli: `rad sync` UX improvements

* Add `--force` flag to force a fetch even if the node is not a seed.
* Don't fail if we can't connect to a node during a fetch.
* More accurately exit when we hit our replica count.
This commit is contained in:
cloudhead 2024-04-09 10:43:35 +02:00
parent e9b79fb6d0
commit 2eb665678e
No known key found for this signature in database
4 changed files with 149 additions and 25 deletions

View File

@ -21,7 +21,8 @@ $ rad sync --announce --timeout 3
✓ Synced with 1 node(s) ✓ Synced with 1 node(s)
``` ```
Bob can now fetch the private repo: Bob can now fetch the private repo without specifying a seed, because he knows
that alice has the repo after she announced her refs:
``` ~bob ``` ~bob
$ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch $ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch

View File

@ -0,0 +1,57 @@
Alice allows Bob to fetch this repo, but doesn't announce it, which means
that Bob needs to know to fetch it from Alice.
``` ~alice
$ rad id update --title "Allow Bob" --description "" --allow did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk -q
[..]
```
First, Bob seeds the repo.
``` ~bob
$ rad seed rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --no-fetch
[..]
```
If Bob just tries to fetch it without specifying seeds, he gets an error:
``` ~bob
$ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch
✗ Error: no seeds found for rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu
```
If he specifies a seed that isn't in his routing table, he gets a warning and
an error:
``` ~bob
$ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch --seed z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi
! Warning: node z6MknSL…StBU8Vi is not connected or seeding.. skipping
✗ Error: no seeds found for rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu
```
For this to work, Bob has to force the sync:
``` ~bob
$ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch --seed z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --force
✓ Fetching rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu from z6MknSL…StBU8Vi..
✓ Fetched repository from 1 seed(s)
```
``` ~bob
$ rad ls --private --all
╭───────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ Name RID Visibility Head Description │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ heartwood rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu private f2de534 radicle heartwood protocol & stack │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────╯
```
Note that if multiple seeds are specified, the command succeeds as long as one
seed succeeds.
``` ~bob
$ rad sync rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu --fetch --seed z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi --seed z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx --force
✓ Fetching rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu from z6MknSL…StBU8Vi..
✗ Fetching rad:z2ug5mwNKZB8KGpBDRTrWHAMbvHCu from z6MkwPU…Ug2mVvx.. error: session does not exist; cannot initiate fetch
✓ Fetched repository from 1 seed(s)
```

View File

@ -1,5 +1,6 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::BTreeSet; use std::collections::BTreeSet;
use std::collections::VecDeque;
use std::ffi::OsString; use std::ffi::OsString;
use std::str::FromStr; use std::str::FromStr;
use std::time; use std::time;
@ -42,6 +43,9 @@ Usage
When `--fetch` is specified, any number of seeds may be given When `--fetch` is specified, any number of seeds may be given
using the `--seed` option, eg. `--seed <nid>@<addr>:<port>`. using the `--seed` option, eg. `--seed <nid>@<addr>:<port>`.
To force a fetch even if there is no route to a seed (as is the case for
private repositories), `--force` can be used.
When `--replicas` is specified, the given replication factor will try When `--replicas` is specified, the given replication factor will try
to be matched. For example, `--replicas 5` will sync with 5 seeds. to be matched. For example, `--replicas 5` will sync with 5 seeds.
@ -61,6 +65,7 @@ Options
-f, --fetch Turn on fetching (default: true) -f, --fetch Turn on fetching (default: true)
-a, --announce Turn on ref announcing (default: true) -a, --announce Turn on ref announcing (default: true)
-i, --inventory Turn on inventory announcing (default: false) -i, --inventory Turn on inventory announcing (default: false)
--force Force fetches from unknown seeds (default: false)
--timeout <secs> How many seconds to wait while syncing --timeout <secs> How many seconds to wait while syncing
--seed <nid> Sync with the given node (may be specified multiple times) --seed <nid> Sync with the given node (may be specified multiple times)
-r, --replicas <count> Sync with a specific number of seeds -r, --replicas <count> Sync with a specific number of seeds
@ -144,6 +149,7 @@ impl Args for Options {
let mut fetch = false; let mut fetch = false;
let mut announce = false; let mut announce = false;
let mut inventory = false; let mut inventory = false;
let mut force = false;
let mut debug = false; let mut debug = false;
let mut replicas = None; let mut replicas = None;
let mut seeds = BTreeSet::new(); let mut seeds = BTreeSet::new();
@ -161,6 +167,9 @@ impl Args for Options {
Long("fetch") | Short('f') => { Long("fetch") | Short('f') => {
fetch = true; fetch = true;
} }
Long("force") => {
force = true;
}
Long("replicas") | Short('r') => { Long("replicas") | Short('r') => {
let val = parser.value()?; let val = parser.value()?;
let count = term::args::number(&val)?; let count = term::args::number(&val)?;
@ -209,8 +218,10 @@ impl Args for Options {
} }
} }
let sync = if inventory && (fetch || announce) { let sync = if inventory && (fetch || announce || force) {
anyhow::bail!("`--inventory` cannot be used with `--fetch` or `--announce`"); anyhow::bail!(
"`--inventory` cannot be used with `--fetch` or `--announce` or `--force`"
);
} else if inventory { } else if inventory {
SyncMode::Inventory SyncMode::Inventory
} else { } else {
@ -219,12 +230,16 @@ impl Args for Options {
(true, false) => SyncDirection::Fetch, (true, false) => SyncDirection::Fetch,
(false, true) => SyncDirection::Announce, (false, true) => SyncDirection::Announce,
}; };
if direction == SyncDirection::Announce && force {
anyhow::bail!("`--force` cannot be used without `--fetch`");
}
let settings = if seeds.is_empty() { let settings = if seeds.is_empty() {
SyncSettings::from_replicas(replicas.unwrap_or(3)) SyncSettings::from_replicas(replicas.unwrap_or(3))
} else { } else {
SyncSettings::from_seeds(seeds) SyncSettings::from_seeds(seeds)
} }
.timeout(timeout); .timeout(timeout)
.force(force);
SyncMode::Repo { SyncMode::Repo {
settings, settings,
@ -431,40 +446,49 @@ pub fn fetch(
let local = node.nid()?; let local = node.nid()?;
// Get seeds. This consults the local routing table only. // Get seeds. This consults the local routing table only.
let seeds = node.seeds(rid)?; let seeds = node.seeds(rid)?;
// Target replicas, clamped by the maximum replicas possible. // Target replicas, clamped by the maximum replicas possible,
let replicas = settings // unless `force` is true.
.replicas let replicas = if settings.force {
.min(seeds.iter().filter(|s| s.nid != local).count()); settings.replicas
} else {
settings
.replicas
.min(seeds.iter().filter(|s| s.nid != local).count())
};
let mut results = FetchResults::default(); let mut results = FetchResults::default();
let (connected, mut disconnected) = seeds.partition(); let (connected, mut disconnected) = seeds.partition();
// Fetch from specified seeds. // Fetch from specified seeds.
for nid in &settings.seeds { for nid in &settings.seeds {
if !seeds.is_connected(nid) && !settings.force { if !seeds.is_connected(nid) && !settings.force {
term::warning(format!("node {nid} is not connected or seeding.. skipping")); term::warning(format!(
"node {} is not connected or seeding.. skipping",
term::format::node(nid)
));
continue; continue;
} }
let result = fetch_from(rid, nid, settings.timeout, node)?; let result = fetch_from(rid, nid, settings.timeout, node)?;
results.push(*nid, result); results.push(*nid, result);
}
if results.success().count() >= replicas { if results.success().count() >= replicas {
return Ok(results); return Ok(results);
}
} }
// Fetch from connected seeds. // Fetch from connected seeds.
let connected = connected let mut connected = connected
.into_iter() .into_iter()
.filter(|c| !results.contains(&c.nid)) .filter(|c| !results.contains(&c.nid))
.map(|c| c.nid) .map(|c| c.nid)
.take(replicas) .take(replicas)
.collect::<Vec<_>>(); .collect::<VecDeque<_>>();
for nid in connected { while results.success().count() < replicas {
let Some(nid) = connected.pop_front() else {
break;
};
let result = fetch_from(rid, &nid, settings.timeout, node)?; let result = fetch_from(rid, &nid, settings.timeout, node)?;
results.push(nid, result); results.push(nid, result);
} }
if results.success().count() >= replicas {
return Ok(results);
}
// Try to connect to disconnected seeds and fetch from them. // Try to connect to disconnected seeds and fetch from them.
while results.success().count() < replicas { while results.success().count() < replicas {
@ -480,7 +504,7 @@ pub fn fetch(
seed.addrs.into_iter().map(|ka| ka.addr), seed.addrs.into_iter().map(|ka| ka.addr),
settings.timeout, settings.timeout,
node, node,
)? { ) {
let result = fetch_from(rid, &seed.nid, settings.timeout, node)?; let result = fetch_from(rid, &seed.nid, settings.timeout, node)?;
results.push(seed.nid, result); results.push(seed.nid, result);
} }
@ -494,7 +518,7 @@ fn connect(
addrs: impl Iterator<Item = node::Address>, addrs: impl Iterator<Item = node::Address>,
timeout: time::Duration, timeout: time::Duration,
node: &mut Node, node: &mut Node,
) -> Result<bool, node::Error> { ) -> bool {
// Try all addresses until one succeeds. // Try all addresses until one succeeds.
for addr in addrs { for addr in addrs {
let spinner = term::spinner(format!( let spinner = term::spinner(format!(
@ -509,20 +533,24 @@ fn connect(
persistent: false, persistent: false,
timeout, timeout,
}, },
)?; );
match cr { match cr {
node::ConnectResult::Connected => { Ok(node::ConnectResult::Connected) => {
spinner.finish(); spinner.finish();
return Ok(true); return true;
} }
node::ConnectResult::Disconnected { .. } => { Ok(node::ConnectResult::Disconnected { .. }) => {
spinner.failed(); spinner.failed();
continue; continue;
} }
Err(e) => {
spinner.error(e);
continue;
}
} }
} }
Ok(false) false
} }
fn fetch_from( fn fetch_from(

View File

@ -2016,6 +2016,44 @@ fn rad_init_private() {
.unwrap(); .unwrap();
} }
#[test]
fn rad_init_private_seed() {
let mut environment = Environment::new();
let alice = environment.node(Config::test(Alias::new("alice")));
let bob = environment.node(Config::test(Alias::new("bob")));
let working = environment.tmp().join("working");
fixtures::repository(working.join("alice"));
let alice = alice.spawn();
let mut bob = bob.spawn();
test(
"examples/rad-init-private.md",
working.join("alice"),
Some(&alice.home),
[],
)
.unwrap();
bob.connect(&alice).converge([&alice]);
formula(&environment.tmp(), "examples/rad-init-private-seed.md")
.unwrap()
.home(
"alice",
working.join("alice"),
[("RAD_HOME", alice.home.path().display())],
)
.home(
"bob",
bob.home.path(),
[("RAD_HOME", bob.home.path().display())],
)
.run()
.unwrap();
}
#[test] #[test]
fn rad_init_private_clone() { fn rad_init_private_clone() {
let mut environment = Environment::new(); let mut environment = Environment::new();