object_store: add Path::from_url_path #3663
Conversation
object_store/src/path/mod.rs
Outdated
| @@ -162,6 +162,13 @@ impl Path { | |||
| }) | |||
| } | |||
|
|
|||
| /// Parse a url encoded string as a [`Path`], returning a [`Error`] if invalid, | |||
| /// as defined on the docstring for [`Path`] | |||
| pub fn from_url_path(path: impl AsRef<str>) -> Result<Self, Error> { | |||
There was a problem hiding this comment.
What do you think of this taking AsRef<Url>?
There was a problem hiding this comment.
make sense, let me do it later today
ps: then we can name it from_url(url: impl AsRef<URL>) instead?
There was a problem hiding this comment.
ps: then we can name it from_url(url: impl AsRef) instead?
I think I would keep it as from_url_path as it will ignore the rest of the URL
There was a problem hiding this comment.
turn out existing code have
arrow-rs/object_store/src/path/mod.rs
Lines 197 to 210 in d14135d
so do you think it makes more sense to use from_url_path(&str)?
object_store/src/path/mod.rs
Outdated
|
|
||
| /// Parse a url encoded string as a [`Path`], returning a [`Error`] if invalid, | ||
| /// as defined on the docstring for [`Path`] | ||
| pub fn from_url_path(path: &str) -> Result<Self, Error> { |
There was a problem hiding this comment.
I was thinking to define as impl AsRef<str>, but
as_bytesmethod not found inimpl AsRef<str>NonUnicodeSnafu { path }: the traitFrom<impl AsRef<str>>is not implemented forString
object_store/src/path/mod.rs
Outdated
|
|
||
| /// Parse a url encoded string as a [`Path`], returning a [`Error`] if invalid, | ||
| /// as defined on the docstring for [`Path`] | ||
| pub fn from_url_path(path: &str) -> Result<Self, Error> { |
There was a problem hiding this comment.
| pub fn from_url_path(path: &str) -> Result<Self, Error> { | |
| pub fn from_url_path(path: impl AsRef<str>) -> Result<Self, Error> { | |
| let path = path.as_ref() |
|
I took the liberty of patching this slightly to get it in, thank you 👍 |
|
Benchmark runs are scheduled for baseline = 9131c30 and contender = fb11792. fb11792 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3651
Rationale for this change
#3651 (comment)
This would simplify user usage of Path, e.g.
What changes are included in this PR?
Implement new method
from_url_pathAre there any user-facing changes?
New method is provided, no impact to existing methods