uv
uv copied to clipboard
astral-sh
`RegistryClientBuilder` should support creation from `BaseClientBuilder`
I can take this one, as I'm interested in learning and contributing!
Awesome! Let me know if you want some guidance on the goals. I pointed out the problem we're trying to solve over in https://github.com/astral-sh/uv/pull/4326#discussion_r1640139844 but didn't provide much detail.
Hey @zanieb ! I think I may take you up on that 😅 . I've been pondering it - we could add another field to the struct:
base_client_builder: Option<BaseClientBuilder<'a>>
then do something like the following in the build function:
let mut builder = self.base_client_builder.unwrap_or_else(BaseClientBuilder::new);
...
let client = builder
.retries(self.retries.unwrap_or(builder.retries))
.connectivity(self.connectivity.unwrap_or(builder.connectivity))
.native_tls(self.native_tls.unwrap_or(builder.native_tls))
.keyring(self.keyring.unwrap_or(builder.keyring))
.build();
but I'm not sure if that's clunky or over-engineering for the duplication, so I would love your feedback (this is also my first time dabbling in Rust, so I may be trying to fit a square peg into a circular hole with this approach).
I think I'd add a base_client_builder field to RegistryClientBuilder but I wouldn't make it an Option. I think we'd create it immediately in RegistryClientBuilder::new then mutate it in the relevant helpers (e.g. when changing native_tls) rather than storing all that state on the RegistryClientBuilder directly. Does that make sense?
Next you can add a From implementation to make it easy to create a registry client builder from an existing base
impl From<BaseClientBuilder> for RegistryClientBuilder ...
Thank you! I understand now - I think I have the updates in place I can push up and make a PR soon. One gap for adding the trait is RegistryClientBuilder needs a cache and I don't believe there is a default value to use e.g;
impl<'a> From<BaseClientBuilder<'a>> for RegistryClientBuilder<'a> {
fn from(value: BaseClientBuilder) -> Self {
Self {
index_urls: IndexUrls::default(),
index_strategy: IndexStrategy::default(),
cache: ..., // Unsure how to approach this unless we add a separate helper function or trait
base_client_builder: BaseClientBuilder::default(),
client: None,
markers: None,
platform: None,
}
}
}