make better use of namespace storage

[macovedj]

Rather than updating the state at the beginning of a client interaction to use a specific url, the client expects to check the namespace storage for a url and pass it as an arg, better enabling consumers to fetch packages across multiple namespaces.

There is also updated logic for encouraging standard ways of retrying with Warg-Hint-Header responses from the registry after using the hint to update local namespace mappings.

[lann]

Only maybe-blocker is question about error handling.

[lann]

nit: You'd typically use match here:

Suggested change

	let namespace_state = op.state.namespace_state(namespace);
	if let Ok(Some(warg_protocol::operator::NamespaceState::Imported { registry })) =
	namespace_state
	{
	return Ok(Some(RegistryDomain::from_str(registry)?));
	} else if let Ok(Some(warg_protocol::operator::NamespaceState::Defined)) =
	namespace_state
	{
	return Ok(None);
	};
	match op.state.namespace_state(namespace) {
	Ok(Some(warg_protocol::operator::NamespaceState::Imported { registry })) => {
	return Ok(Some(RegistryDomain::from_str(registry)?));
	}
	Ok(Some(warg_protocol::operator::NamespaceState::Defined)) => {
	return Ok(None);
	}
	_ => (),
	}

[lann]

Are Errs expected here? Should probably at least log them if not propagating, unless there is a specific reason to completely suppress.

[lann]

nit: There is a nice pattern for this:

Suggested change

	let namespace_packages = namespaced.get_mut(namespace);
	if let Some(nm_pkgs) = namespace_packages {
	nm_pkgs.push(package);
	} else {
	namespaced.insert(namespace, vec![package]);
	}
	namespaced.entry(namespace).or_default().push(package);

[lann]

Can this go outside the match? The commands that don't support retry shouldn't be returning this error anyway and can just ignore the retry variable?

[lann]

minor: its difficult to see that this if block always returns

Suggested change

	match op.state.namespace_state(namespace) {
	Ok(Some(warg_protocol::operator::NamespaceState::Imported { registry })) => {
	return Ok(Some(RegistryDomain::from_str(registry)?));
	}
	Ok(Some(warg_protocol::operator::NamespaceState::Defined)) => {
	return Ok(None);
	}
	Ok(None) => return Ok(None),
	Err(e) => {
	return Err(ClientError::SimilarNamespace {
	namespace: namespace.to_string(),
	e: e.to_string(),
	});
	}
	}
	return match op.state.namespace_state(namespace) {
	Ok(Some(warg_protocol::operator::NamespaceState::Imported { registry })) => {
	Ok(Some(RegistryDomain::from_str(registry)?))
	}
	Ok(Some(warg_protocol::operator::NamespaceState::Defined)) => {
	Ok(None)
	}
	Ok(None) => return Ok(None),
	Err(e) => {
	Err(ClientError::SimilarNamespace {
	namespace: namespace.to_string(),
	e: e.to_string(),
	})
	}
	}

[lann]

Are we not enforcing lowercase namespaces? I think we should be... WebAssembly/component-model#338

[calvinrp]

Yeah, that is outstanding. Probably better as a separate PR. Found the open issue #245

[lann]

This doesn't appear to do what it claims to do?

[calvinrp]

Collaborated with @macovedj to rework these changes into PR #268 which was merged. Thus closing this PR.