moved the keyring functions back into warg_client

[calvinrp]

This allows us to have a simpler FileSystemClient::new_with_default_config() that gets the expected auth_token.

Would simplify bytecodealliance/cargo-component#288, bytecodealliance/wac#103, bytecodealliance/wasm-pkg-tools#6, esoterra/wow#3

This:

        let config = warg_client::Config::from_default_file()?.unwrap_or_default();
        let url = url.or(config.home_url.as_deref());
        let auth_token = if config.keyring_auth && url.is_some() {
            get_auth_token(&RegistryUrl::new(url.unwrap())?)?
        } else {
            None
        };
        Ok(Self {
            client: FileSystemClient::new_with_config(url, &config, auth_token)?,
        })

would change to:

        Ok(Self {
            client: FileSystemClient::new_with_default_config(url)?,
        })

[lann]

Partial review; I suspect this won't compile with cargo build --no-default-features

[lann]

I don't know that we actually need to make this optional for the binary do we? Actually, I didn't really think about this until now: it probably doesn't make sense to have any of these features; what does the postgres feature even mean here?

[calvinrp]

The postgres feature is for the warg-server switches between using an in-memory vs Postgres backing when setting the feature flag. That probably still makes sense for now.

The other keyring and cli-interactive features might make sense to have the option of compiling without those implementations in the warg-client.

[lann]

These features are just for the crate defined in this file: warg-cli

[calvinrp]

Ah, you mean we can drop postgres feature in the top-level Cargo.toml but keep it in the crates/server/Cargo.toml?

[lann]

Yeah, most of the root Cargo.toml is (somewhat confusingly) completely independent of other workspace members.

[calvinrp]

Removing postgres from the top-level Cargo.toml appears to affect how I run tests. It might affect the test runner, but would need to double check. But doesn't seem urgent.

[calvinrp]

For the time being, we might want to keep the top-level postgres feature. The test runner is setup expecting it. https://github.com/bytecodealliance/registry/blob/main/ci/run-postgres-tests.sh#L26

[lann]

Ok; are the keyring and cli-interactive features needed?

[calvinrp]

Well, in theory, if you don't want those features, it will be useful to have them behind a feature flag. But not entirely sure.

[lann]

Ok; are the keyring and cli-interactive features needed?

[dfoxfranke]

Was it intentional that you're now passing None for the auth token (and that you deleted the now-unused auth_token method below)?

[calvinrp]

Yes, I was trying to simplify the burden on external crates getting the correct auth_token. So None gets the correct one, https://github.com/bytecodealliance/registry/blob/main/crates/client/src/lib.rs#L1403-L1405

Also, introduced new_with_default_config() to simplify, https://github.com/bytecodealliance/registry/blob/main/crates/client/src/lib.rs#L1428-L1430