Skip to content

Commit 83a9512

Browse files
authored
re_uri: Use correct default port (51234, not 80) (#9468)
This makes it more ergonomic to connect to redap. Also: use more idiomatic `FromStr` instead of `TryFrom<&str>`. I've opened a clippy lint about this: rust-lang/rust-clippy#14522 Review each commit seperately.
1 parent 8f97013 commit 83a9512

File tree

12 files changed

+80
-58
lines changed

12 files changed

+80
-58
lines changed

crates/store/re_data_source/src/data_source.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ impl DataSource {
102102
}
103103
}
104104

105-
if let Ok(endpoint) = re_uri::RedapUri::try_from(uri.as_str()) {
105+
if let Ok(endpoint) = uri.as_str().parse::<re_uri::RedapUri>() {
106106
return Self::RerunGrpcStream(endpoint);
107107
}
108108

crates/store/re_log_types/src/path/entity_path_filter.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,10 @@ pub struct EntityPathFilter {
6161
rules: BTreeMap<EntityPathRule, RuleEffect>,
6262
}
6363

64-
// Note: it's not possible to implement that for `S: AsRef<str>` because this conflicts with some
65-
// blanket implementation in `core` :(
66-
impl TryFrom<&str> for EntityPathFilter {
67-
type Error = EntityPathFilterError;
64+
impl std::str::FromStr for EntityPathFilter {
65+
type Err = EntityPathFilterError;
6866

69-
fn try_from(value: &str) -> Result<Self, Self::Error> {
67+
fn from_str(value: &str) -> Result<Self, Self::Err> {
7068
Self::parse_strict(value)
7169
}
7270
}

crates/top/re_sdk/src/recording_stream.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,7 @@ impl RecordingStreamBuilder {
384384
let (enabled, store_info, properties, batcher_config) = self.into_args();
385385
if enabled {
386386
let url: String = url.into();
387-
let re_uri::RedapUri::Proxy(endpoint) = re_uri::RedapUri::try_from(url.as_str())?
388-
else {
387+
let re_uri::RedapUri::Proxy(endpoint) = url.as_str().parse()? else {
389388
return Err(RecordingStreamError::NotAProxyEndpoint);
390389
};
391390

@@ -1809,7 +1808,7 @@ impl RecordingStream {
18091808
}
18101809

18111810
let url: String = url.into();
1812-
let re_uri::RedapUri::Proxy(endpoint) = re_uri::RedapUri::try_from(url.as_str())? else {
1811+
let re_uri::RedapUri::Proxy(endpoint) = url.as_str().parse()? else {
18131812
return Err(RecordingStreamError::NotAProxyEndpoint);
18141813
};
18151814

crates/top/rerun/src/commands/entrypoint.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -762,8 +762,7 @@ fn run_impl(
762762
#[cfg(feature = "server")]
763763
if let Some(url) = args.connect {
764764
let url = url.unwrap_or_else(|| format!("rerun+http://{server_addr}/proxy"));
765-
let re_uri::RedapUri::Proxy(endpoint) = re_uri::RedapUri::try_from(url.as_str())?
766-
else {
765+
let re_uri::RedapUri::Proxy(endpoint) = url.as_str().parse()? else {
767766
anyhow::bail!("expected `/proxy` endpoint");
768767
};
769768
let rx = re_sdk::external::re_grpc_client::message_proxy::stream(endpoint, None);
@@ -857,8 +856,7 @@ fn run_impl(
857856
format!("rerun+http://{server_addr}/proxy")
858857
};
859858

860-
let re_uri::RedapUri::Proxy(endpoint) = re_uri::RedapUri::try_from(url.as_str())?
861-
else {
859+
let re_uri::RedapUri::Proxy(endpoint) = url.as_str().parse()? else {
862860
anyhow::bail!("expected `/proxy` endpoint");
863861
};
864862

crates/utils/re_uri/src/redap_uri.rs

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::str::FromStr as _;
1+
use std::str::FromStr;
22

33
use re_log_types::{TimeCell, TimeInt};
44

@@ -27,10 +27,10 @@ impl std::fmt::Display for TimeRange {
2727
}
2828
}
2929

30-
impl TryFrom<&str> for TimeRange {
31-
type Error = Error;
30+
impl FromStr for TimeRange {
31+
type Err = Error;
3232

33-
fn try_from(value: &str) -> Result<Self, crate::Error> {
33+
fn from_str(value: &str) -> Result<Self, Self::Err> {
3434
let (timeline, range) = value
3535
.split_once('@')
3636
.ok_or_else(|| Error::InvalidTimeRange("Missing @".to_owned()))?;
@@ -111,10 +111,10 @@ impl Scheme {
111111
}
112112
}
113113

114-
impl TryFrom<&str> for Scheme {
115-
type Error = Error;
114+
impl FromStr for Scheme {
115+
type Err = Error;
116116

117-
fn try_from(url: &str) -> Result<Self, crate::Error> {
117+
fn from_str(url: &str) -> Result<Self, Self::Err> {
118118
if url.starts_with("rerun://") {
119119
Ok(Self::Rerun)
120120
} else if url.starts_with("rerun+http://") {
@@ -158,13 +158,18 @@ impl crate::Origin {
158158
/// Parses a URL and returns the [`crate::Origin`] and the canonical URL (i.e. one that
159159
/// starts with `http://` or `https://`).
160160
fn replace_and_parse(value: &str) -> Result<(crate::Origin, url::Url), Error> {
161-
let scheme = Scheme::try_from(value)?;
161+
let scheme = Scheme::from_str(value)?;
162162
let rewritten = scheme.canonical_url(value);
163163

164164
// We have to first rewrite the endpoint, because `Url` does not allow
165165
// `.set_scheme()` for non-opaque origins, nor does it return a proper
166166
// `Origin` in that case.
167-
let http_url = url::Url::parse(&rewritten)?;
167+
let mut http_url = url::Url::parse(&rewritten)?;
168+
169+
if http_url.port().is_none() {
170+
// If no port is specified, we assume the default redap port:
171+
http_url.set_port(Some(51234)).ok();
172+
}
168173

169174
let url::Origin::Tuple(_, host, port) = http_url.origin() else {
170175
return Err(Error::UnexpectedOpaqueOrigin(value.to_owned()));
@@ -175,10 +180,10 @@ fn replace_and_parse(value: &str) -> Result<(crate::Origin, url::Url), Error> {
175180
Ok((origin, http_url))
176181
}
177182

178-
impl TryFrom<&str> for crate::Origin {
179-
type Error = Error;
183+
impl FromStr for crate::Origin {
184+
type Err = Error;
180185

181-
fn try_from(value: &str) -> Result<Self, crate::Error> {
186+
fn from_str(value: &str) -> Result<Self, Self::Err> {
182187
replace_and_parse(value).map(|(origin, _)| origin)
183188
}
184189
}
@@ -213,15 +218,7 @@ impl std::fmt::Display for RedapUri {
213218
impl std::str::FromStr for RedapUri {
214219
type Err = Error;
215220

216-
fn from_str(s: &str) -> Result<Self, Self::Err> {
217-
Self::try_from(s)
218-
}
219-
}
220-
221-
impl TryFrom<&str> for RedapUri {
222-
type Error = Error;
223-
224-
fn try_from(value: &str) -> Result<Self, crate::Error> {
221+
fn from_str(value: &str) -> Result<Self, Self::Err> {
225222
let (origin, http_url) = replace_and_parse(value)?;
226223

227224
// :warning: We limit the amount of segments, which might need to be
@@ -236,7 +233,7 @@ impl TryFrom<&str> for RedapUri {
236233
let time_range = http_url
237234
.query_pairs()
238235
.find(|(key, _)| key == TimeRange::QUERY_KEY)
239-
.map(|(_, value)| TimeRange::try_from(value.as_ref()));
236+
.map(|(_, value)| TimeRange::from_str(value.as_ref()));
240237

241238
match segments.as_slice() {
242239
["proxy"] => Ok(Self::Proxy(ProxyEndpoint::new(origin))),
@@ -306,7 +303,7 @@ mod tests {
306303
fn test_dataset_data_url_to_address() {
307304
let url =
308305
"rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data?partition_id=pid";
309-
let address: RedapUri = url.try_into().unwrap();
306+
let address: RedapUri = url.parse().unwrap();
310307

311308
let RedapUri::DatasetData(DatasetDataEndpoint {
312309
origin,
@@ -332,7 +329,7 @@ mod tests {
332329
#[test]
333330
fn test_dataset_data_url_time_range_sequence_to_address() {
334331
let url = "rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data?partition_id=pid&[email protected]";
335-
let address: RedapUri = url.try_into().unwrap();
332+
let address: RedapUri = url.parse().unwrap();
336333

337334
let RedapUri::DatasetData(DatasetDataEndpoint {
338335
origin,
@@ -367,7 +364,7 @@ mod tests {
367364
#[test]
368365
fn test_dataset_data_url_time_range_timepoint_to_address() {
369366
let url = "rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data?partition_id=pid&time_range=log_time@2022-01-01T00:00:03.123456789Z..2022-01-01T00:00:13.123456789Z";
370-
let address: RedapUri = url.try_into().unwrap();
367+
let address: RedapUri = url.parse().unwrap();
371368

372369
let RedapUri::DatasetData(DatasetDataEndpoint {
373370
origin,
@@ -409,7 +406,7 @@ mod tests {
409406
"rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data?partition_id=pid&[email protected]",
410407
"rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data?partition_id=pid&[email protected]",
411408
] {
412-
let address: RedapUri = url.try_into().unwrap();
409+
let address: RedapUri = url.parse().unwrap();
413410

414411
let RedapUri::DatasetData(DatasetDataEndpoint {
415412
origin,
@@ -446,13 +443,13 @@ mod tests {
446443
fn test_dataset_data_url_missing_partition_id() {
447444
let url = "rerun://127.0.0.1:1234/dataset/1830B33B45B963E7774455beb91701ae/data";
448445

449-
assert!(RedapUri::try_from(url).is_err());
446+
assert!(RedapUri::from_str(url).is_err());
450447
}
451448

452449
#[test]
453450
fn test_http_catalog_url_to_address() {
454451
let url = "rerun+http://127.0.0.1:50051/catalog";
455-
let address: RedapUri = url.try_into().unwrap();
452+
let address: RedapUri = url.parse().unwrap();
456453
assert!(matches!(
457454
address,
458455
RedapUri::Catalog(CatalogEndpoint {
@@ -468,7 +465,7 @@ mod tests {
468465
#[test]
469466
fn test_https_catalog_url_to_address() {
470467
let url = "rerun+https://127.0.0.1:50051/catalog";
471-
let address: RedapUri = url.try_into().unwrap();
468+
let address: RedapUri = url.parse().unwrap();
472469

473470
assert!(matches!(
474471
address,
@@ -485,7 +482,7 @@ mod tests {
485482
#[test]
486483
fn test_localhost_url() {
487484
let url = "rerun+http://localhost:51234/catalog";
488-
let address = RedapUri::try_from(url).unwrap();
485+
let address = RedapUri::from_str(url).unwrap();
489486

490487
assert_eq!(
491488
address,
@@ -502,7 +499,7 @@ mod tests {
502499
#[test]
503500
fn test_invalid_url() {
504501
let url = "http://wrong-scheme:1234/recording/12345";
505-
let address: Result<RedapUri, _> = url.try_into();
502+
let address: Result<RedapUri, _> = url.parse();
506503

507504
assert!(matches!(
508505
address.unwrap_err(),
@@ -513,7 +510,7 @@ mod tests {
513510
#[test]
514511
fn test_invalid_path() {
515512
let url = "rerun://0.0.0.0:51234/redap/recordings/12345";
516-
let address: Result<RedapUri, _> = url.try_into();
513+
let address: Result<RedapUri, _> = url.parse();
517514

518515
assert!(matches!(
519516
address.unwrap_err(),
@@ -524,7 +521,7 @@ mod tests {
524521
#[test]
525522
fn test_proxy_endpoint() {
526523
let url = "rerun://localhost:51234/proxy";
527-
let address: Result<RedapUri, _> = url.try_into();
524+
let address: Result<RedapUri, _> = url.parse();
528525

529526
let expected = RedapUri::Proxy(ProxyEndpoint {
530527
origin: Origin {
@@ -537,15 +534,15 @@ mod tests {
537534
assert_eq!(address.unwrap(), expected);
538535

539536
let url = "rerun://localhost:51234/proxy/";
540-
let address: Result<RedapUri, _> = url.try_into();
537+
let address: Result<RedapUri, _> = url.parse();
541538

542539
assert_eq!(address.unwrap(), expected);
543540
}
544541

545542
#[test]
546543
fn test_catalog_default() {
547544
let url = "rerun://localhost:51234";
548-
let address: Result<RedapUri, _> = url.try_into();
545+
let address: Result<RedapUri, _> = url.parse();
549546

550547
let expected = RedapUri::Catalog(CatalogEndpoint {
551548
origin: Origin {
@@ -558,8 +555,38 @@ mod tests {
558555
assert_eq!(address.unwrap(), expected);
559556

560557
let url = "rerun://localhost:51234/";
561-
let address: Result<RedapUri, _> = url.try_into();
558+
let address: Result<RedapUri, _> = url.parse();
562559

563560
assert_eq!(address.unwrap(), expected);
564561
}
562+
563+
#[test]
564+
fn test_default_port() {
565+
let url = "rerun://localhost";
566+
567+
let expected = RedapUri::Catalog(CatalogEndpoint {
568+
origin: Origin {
569+
scheme: Scheme::Rerun,
570+
host: url::Host::Domain("localhost".to_owned()),
571+
port: 51234,
572+
},
573+
});
574+
575+
assert_eq!(url.parse::<RedapUri>().unwrap(), expected);
576+
}
577+
578+
#[test]
579+
fn test_custom_port() {
580+
let url = "rerun://localhost:123";
581+
582+
let expected = RedapUri::Catalog(CatalogEndpoint {
583+
origin: Origin {
584+
scheme: Scheme::Rerun,
585+
host: url::Host::Domain("localhost".to_owned()),
586+
port: 123,
587+
},
588+
});
589+
590+
assert_eq!(url.parse::<RedapUri>().unwrap(), expected);
591+
}
565592
}

crates/viewer/re_blueprint_tree/tests/blueprint_tree_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ fn setup_filter_test(query: Option<&str>) -> (TestContext, BlueprintTree) {
152152
re_view_spatial::SpatialView3D::identifier(),
153153
RecommendedView {
154154
origin: "/path/to".into(),
155-
query_filter: "+ /**".try_into().expect("valid entity path filter"),
155+
query_filter: "+ /**".parse().expect("valid entity path filter"),
156156
},
157157
)),
158158
None,

crates/viewer/re_blueprint_tree/tests/view_structure_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ fn test_context(test_case: &TestCase) -> TestContext {
128128
origin: test_case.origin.clone(),
129129
query_filter: test_case
130130
.entity_filter
131-
.try_into()
131+
.parse()
132132
.expect("invalid entity filter"),
133133
},
134134
ViewId::hashed_from_str(VIEW_ID),

crates/viewer/re_view_spatial/tests/transform_clamping.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,15 @@ fn setup_blueprint(test_context: &mut TestContext) -> (ViewId, ViewId) {
163163
re_view_spatial::SpatialView3D::identifier(),
164164
RecommendedView {
165165
origin: "/boxes".into(),
166-
query_filter: "+ $origin/**".try_into().unwrap(),
166+
query_filter: "+ $origin/**".parse().unwrap(),
167167
},
168168
);
169169

170170
let view_blueprint_spheres = ViewBlueprint::new(
171171
re_view_spatial::SpatialView3D::identifier(),
172172
RecommendedView {
173173
origin: "/spheres".into(),
174-
query_filter: "+ $origin/**".try_into().unwrap(),
174+
query_filter: "+ $origin/**".parse().unwrap(),
175175
},
176176
);
177177

crates/viewer/re_viewer/src/app_state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ fn check_for_clicked_hyperlinks(ctx: &ViewerContext<'_>) {
895895
ctx.egui_ctx().output_mut(|o| {
896896
o.commands.retain_mut(|command| {
897897
if let egui::OutputCommand::OpenUrl(open_url) = command {
898-
let is_rerun_url = re_uri::RedapUri::try_from(open_url.url.as_ref()).is_ok();
898+
let is_rerun_url = open_url.url.parse::<re_uri::RedapUri>().is_ok();
899899

900900
if is_rerun_url {
901901
let data_source = re_data_source::DataSource::from_uri(

crates/viewer/re_viewer/src/web_tools.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ enum EndpointCategory {
9999

100100
impl EndpointCategory {
101101
fn categorize_uri(uri: String) -> Self {
102-
if let Ok(uri) = re_uri::RedapUri::try_from(uri.as_ref()) {
102+
if let Ok(uri) = uri.parse() {
103103
return Self::RerunGrpcStream(uri);
104104
}
105105

examples/rust/dataframe_query/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use itertools::Itertools;
44

55
use rerun::{
6-
dataframe::{EntityPathFilter, QueryEngine, QueryExpression, SparseFillStrategy, TimelineName},
6+
dataframe::{QueryEngine, QueryExpression, SparseFillStrategy, TimelineName},
77
external::{arrow, re_format_arrow::format_record_batch},
88
ChunkStoreConfig, StoreKind, VersionPolicy,
99
};
@@ -37,7 +37,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
3737
};
3838

3939
let path_to_rrd = get_arg(1);
40-
let entity_path_filter = EntityPathFilter::try_from(args.get(2).map_or("/**", |s| s.as_str()))?;
40+
let entity_path_filter = args.get(2).map_or("/**", |s| s.as_str()).parse()?;
4141
let timeline = TimelineName::log_time();
4242

4343
let engines = QueryEngine::from_rrd_filepath(

rerun_py/src/catalog/catalog_client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl PyCatalogClient {
2626
/// Create a new catalog client object.
2727
#[new]
2828
fn new(py: Python<'_>, addr: String) -> PyResult<Self> {
29-
let origin = re_uri::Origin::try_from(addr.as_str()).map_err(to_py_err)?;
29+
let origin = addr.as_str().parse::<re_uri::Origin>().map_err(to_py_err)?;
3030

3131
let connection = ConnectionHandle::new(py, origin.clone())?;
3232

0 commit comments

Comments
 (0)