-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get referenced obj using dynamic GVK API #1
base: main
Are you sure you want to change the base?
Get referenced obj using dynamic GVK API #1
Conversation
Signed-off-by: Nitish Malhotra <[email protected]>
pub async fn resolve_uri( | ||
&self, | ||
client: kube::Client, | ||
) -> Result<url::Url, Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to keep our error model in-line with kube's by sticking with thiserror
wrappers of Error.
If we add a Variant to knative error with a #[from]
kube-client::Error
then it will automatically implement TryFrom
which gives us back ?
operator without resorting to Box<dyn std::error::Error>
. We may need to drop Clone
and Copy
derives, but that's ok!
Our error can look like this:
use kube::error::Error as KubeError;
#[derive(Error, Debug)]
pub enum Error {
/// Discovery errors
#[error("Error from discovery: {0}")]
Discovery(#[source] DiscoveryError),
/// Kube errors
#[error("Error from kube: {0}")]
KubeError(#[from] KubeError),
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the Discovery
Variant is admittedly not very thought out. Feel free to scratch or rework it when we add the actual Errors that can occur during KReference
uri resolution!
Ok(url) | ||
} | ||
(None, Some(ref uri)) => Ok(uri.clone()), | ||
(None, None) => Err(Error::Discovery(DiscoveryError::EmptyDestination)) | ||
(None, None) => Err(Box::new(Error::Discovery(DiscoveryError::EmptyDestination))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the box once we add the kube::Error
Variant to our error
duckv1.AddressableType
and get URL fromstatus
. This probably requires the type to be defined and implemented first.Signed-off-by: Nitish Malhotra [email protected]