You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "prantogg (via GitHub)" <gi...@apache.org> on 2025/10/17 22:32:17 UTC

[PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

prantogg opened a new pull request, #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54

   This change switches the Zone dataset source from the Overture Maps public S3 bucket to a mirrored copy hosted on Hugging Face under [apache-sedona/spatialbench](https://huggingface.co/datasets/apache-sedona/spatialbench).


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu merged PR #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on PR #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54#issuecomment-3417808930

   @prantogg please address conflict


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on PR #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54#issuecomment-3420455274

   @prantogg please address conlfict as well


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54#discussion_r2445839378


##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -200,29 +192,39 @@ 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);
+    // 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/{}/omf-division-area-{}/part-{i:05}-{uuid}-c000.zstd.parquet",

Review Comment:
   The format string contains redundant named parameters. The format string uses `{i:05}` and `{uuid}` but then explicitly names them in the arguments with `i = i` and `uuid = PARQUET_UUID`. Either use positional arguments `{0:05}` and `{1}`, or use named parameters `{i:05}` and `{uuid}` without the explicit naming in the arguments.



##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -11,24 +11,16 @@ use datafusion::{
 use crate::plan::DEFAULT_PARQUET_ROW_GROUP_BYTES;
 use datafusion::execution::runtime_env::RuntimeEnv;
 use log::{debug, info};
-use object_store::aws::AmazonS3Builder;
-use object_store::ObjectStore;
+use object_store::http::HttpBuilder;
 use parquet::{
     arrow::ArrowWriter, basic::Compression as ParquetCompression,
     file::properties::WriterProperties,
 };
 use url::Url;
 
 const OVERTURE_RELEASE_DATE: &str = "2025-08-20.1";
-const OVERTURE_S3_BUCKET: &str = "overturemaps-us-west-2";
-const OVERTURE_S3_PREFIX: &str = "release";
-
-fn zones_parquet_url() -> String {
-    format!(
-        "s3://{}/{}/{}/theme=divisions/type=division_area/",
-        OVERTURE_S3_BUCKET, OVERTURE_S3_PREFIX, OVERTURE_RELEASE_DATE
-    )
-}
+const HUGGINGFACE_URL: &str = "https://huggingface.co";

Review Comment:
   The hardcoded commit hash lacks context. Add a comment explaining that this is the Hugging Face dataset commit hash and what happens if the commit is updated (e.g., does PARQUET_PART_COUNT or PARQUET_UUID need to change?).
   ```suggestion
   const HUGGINGFACE_URL: &str = "https://huggingface.co";
   // This is the Hugging Face dataset commit hash for the Overture release.
   // If the dataset is updated and the commit hash changes, you may also need to update
   // related constants such as PARQUET_PART_COUNT or PARQUET_UUID to match the new dataset structure.
   ```



##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -200,29 +192,39 @@ 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);
+    // Parquet parts from Hugging Face (programmatically generated)
+    const PARQUET_PART_COUNT: usize = 4;

Review Comment:
   These magic constants lack explanation. Add comments explaining what PARQUET_PART_COUNT represents (number of sharded parquet files?) and the significance of PARQUET_UUID (is this a file identifier from the Hugging Face dataset?).
   ```suggestion
       // Parquet parts from Hugging Face (programmatically generated)
       // Number of sharded Parquet files (parts) to read from Hugging Face for the dataset.
       const PARQUET_PART_COUNT: usize = 4;
       // Unique identifier for the Parquet file set, corresponding to a specific version/commit in the Hugging Face dataset.
   ```



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: load Overture Divisions from Hugging Face over HTTPS instead of S3 [sedona-spatialbench]

Posted by "jiayuasu (via GitHub)" <gi...@apache.org>.
jiayuasu commented on code in PR #54:
URL: https://github.com/apache/sedona-spatialbench/pull/54#discussion_r2441577935


##########
spatialbench-cli/src/zone_df.rs:
##########
@@ -200,29 +191,38 @@ 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);
+    // 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",

Review Comment:
   Can we pin this to a specific huggingface commit instead of the main: https://huggingface.co/datasets/apache-sedona/spatialbench/commits/main



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org