Copilot commented on code in PR #54:
URL:
https://github.com/apache/sedona-spatialbench/pull/54#discussion_r2441407520
##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -200,29 +191,34 @@ pub async fn generate_zone_parquet(args: ZoneDfArgs) ->
Result<()> {
let rt: Arc<RuntimeEnv> = Arc::new(RuntimeEnvBuilder::new().build()?);
debug!("Built DataFusion runtime environment");
- // Register S3 store for Overture bucket
- let bucket = OVERTURE_S3_BUCKET;
- info!("Registering S3 store for bucket: {}", bucket);
- let s3 = AmazonS3Builder::new()
- .with_bucket_name(bucket)
- .with_skip_signature(true)
- .with_region("us-west-2")
- .build()?;
-
- let s3_url = Url::parse(&format!("s3://{bucket}"))?;
- let s3_store: Arc<dyn ObjectStore> = Arc::new(s3);
- rt.register_object_store(&s3_url, s3_store);
- debug!("Successfully registered S3 object store");
+ // Register HTTPS object store for Hugging Face
+ let hf_store = HttpBuilder::new().with_url(HUGGINGFACE_URL).build()?;
+ let hf_url = Url::parse(HUGGINGFACE_URL)?;
+ rt.register_object_store(&hf_url, Arc::new(hf_store));
+ debug!("Registered HTTPS object store for huggingface.co");
let ctx = SessionContext::new_with_config_rt(SessionConfig::from(cfg), rt);
debug!("Created DataFusion session context");
- let url = zones_parquet_url();
- info!("Reading parquet data from: {}", url);
+ // 4 Parquet parts from Hugging Face
+ let parquet_urls = vec![
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00000-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00001-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00002-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00003-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+ ];
Review Comment:
[nitpick] The four nearly identical URL constructions repeat the base path
and UUID segment, and the hard-coded part count will require manual edits if
the number of Parquet parts changes for future releases. Consider deriving the
list programmatically (e.g., using a range and a base format string: let base =
format!(\"{}/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}\",
HUGGINGFACE_URL, OVERTURE_RELEASE_DATE); then map over 0..part_count to
produce part-{:05}.). This reduces duplication and makes it easier to adjust
when the dataset adds or removes parts.
```suggestion
// Parquet parts from Hugging Face (programmatically generated)
const PARQUET_PART_COUNT: usize = 4;
const PARQUET_UUID: &str = "c998b093-fa14-440c-98f0-bbdb2126ed22";
let parquet_urls: Vec<String> = (0..PARQUET_PART_COUNT)
.map(|i| format!(
"https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-{i:05}-{uuid}-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE,
i = i,
uuid = PARQUET_UUID
))
.collect();
```
##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -200,29 +191,34 @@ pub async fn generate_zone_parquet(args: ZoneDfArgs) ->
Result<()> {
let rt: Arc<RuntimeEnv> = Arc::new(RuntimeEnvBuilder::new().build()?);
debug!("Built DataFusion runtime environment");
- // Register S3 store for Overture bucket
- let bucket = OVERTURE_S3_BUCKET;
- info!("Registering S3 store for bucket: {}", bucket);
- let s3 = AmazonS3Builder::new()
- .with_bucket_name(bucket)
- .with_skip_signature(true)
- .with_region("us-west-2")
- .build()?;
-
- let s3_url = Url::parse(&format!("s3://{bucket}"))?;
- let s3_store: Arc<dyn ObjectStore> = Arc::new(s3);
- rt.register_object_store(&s3_url, s3_store);
- debug!("Successfully registered S3 object store");
+ // Register HTTPS object store for Hugging Face
+ let hf_store = HttpBuilder::new().with_url(HUGGINGFACE_URL).build()?;
+ let hf_url = Url::parse(HUGGINGFACE_URL)?;
+ rt.register_object_store(&hf_url, Arc::new(hf_store));
+ debug!("Registered HTTPS object store for huggingface.co");
let ctx = SessionContext::new_with_config_rt(SessionConfig::from(cfg), rt);
debug!("Created DataFusion session context");
- let url = zones_parquet_url();
- info!("Reading parquet data from: {}", url);
+ // 4 Parquet parts from Hugging Face
+ let parquet_urls = vec![
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00000-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00001-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00002-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+
format!("https://huggingface.co/datasets/apache-sedona/spatialbench/resolve/main/omf-division-area-{}/part-00003-c998b093-fa14-440c-98f0-bbdb2126ed22-c000.zstd.parquet";,
OVERTURE_RELEASE_DATE),
+ ];
+
+ info!(
+ "Reading {} Parquet parts from Hugging Face…",
Review Comment:
[nitpick] The Unicode ellipsis (…) in the log message can hinder simple
ASCII-only grepping or log processing; using '...' improves portability:
\"Reading {} Parquet parts from Hugging Face...\".
```suggestion
"Reading {} Parquet parts from Hugging Face...",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
