chore: removing APIs and deprecation warnings: 0.30.x here we come by rtyler · Pull Request #3962 · delta-io/delta-rs

rtyler · 2025-12-02T14:06:38Z

💣 💥

This is a pretty massive change, regrettably, but moves a tremendous amount of code from the 🦀 from using conventional/goofy &str implementations to using url::Url. This ended up surfacing a number of Url oddities that needed to be accounted for, most notably the semantic importance of a trailing / on our use of Url since we heavily rely on Url::join.

There are additional changes here as a result of a Rust edition upgrade to 2024 and the corresponding lints and clippy checks.

To boot, a lot of deprecated code is hereby removed in this change, thus the big negative number 🔨

codecov · 2025-12-02T14:08:30Z

Codecov Report

❌ Patch coverage is 83.09859% with 180 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.50%. Comparing base (3469c37) to head (80ab73b).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/core/src/kernel/models/actions.rs	0.00%	18 Missing ⚠️
python/src/lib.rs	0.00%	15 Missing ⚠️
crates/lakefs/src/logstore.rs	53.33%	12 Missing and 2 partials ⚠️
crates/aws/src/lib.rs	79.16%	10 Missing ⚠️
crates/core/src/lib.rs	84.84%	0 Missing and 10 partials ⚠️
crates/core/src/kernel/schema/cast/merge_schema.rs	0.00%	7 Missing ⚠️
crates/core/src/operations/write/mod.rs	68.18%	4 Missing and 3 partials ⚠️
crates/test/src/read.rs	76.66%	0 Missing and 7 partials ⚠️
crates/core/src/logstore/mod.rs	93.82%	5 Missing ⚠️
crates/core/src/operations/vacuum.rs	50.00%	5 Missing ⚠️
... and 40 more

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3962       +/-   ##
===========================================
+ Coverage   26.22%   74.50%   +48.28%     
===========================================
  Files         124      152       +28     
  Lines       19885    39952    +20067     
  Branches    19885    39952    +20067     
===========================================
+ Hits         5214    29765    +24551     
+ Misses      14301     8854     -5447     
- Partials      370     1333      +963

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

roeap

first pass over pushed changes. Just one question.

we need some more clippy, but awesome to get rid of some of this.

corwinjoy · 2025-12-03T22:52:39Z

Cool! If you need my help I can rerun somthing like this PR to remove the other clippy warnings.
#3940

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

When using LogStoreConfig it is possible to pass a Url which does allow for safe `join()` operations inside of delta-kernel-rs. This change ensures that we always have a trailing slash before passing things off into kernel Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

This is almost entirely formatting changes, which is a _wee_ bit annoying. There were no practical code changes required however. Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

Every trailing slashes cause all sorts of subtle equivalency confusion when we do things with a Url. It's better for us to normalize everything to always have a trailing slash which makes it easier to join with, and do other things. Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

In this commit I am intentionally introducing the convention in our Rust APIs: * anything named `uri` is expected to take a `AsRef<str>` type which is expected to turn into a `Url` * anything named `url` is expected to take a `Url` type. As such I have renamed a number of our APIs which previously had been converted to use `Url` but were referring to them as `uri` Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

ion-elgreco · 2025-12-14T16:34:38Z

+        match operation_id {
+            Some(op) => self.get_transaction_url(op, self.config.location().to_string()),
+            None => Err(DeltaTableError::InvalidData {
+                violations: vec!["LakeFS must use operation_ids for operations".into()],


Small nit, but the nested error doesn't surface now which gives a hint in which area of the code it happens

roeap

🎉

roeap · 2025-12-14T16:01:06Z

+                            // NOTE: the lack of trailing slashes is a load-bearing implementation
+                            // detail between the Delta/Spark and delta-rs S3DynamoDbLogStore


roeap · 2025-12-14T16:26:25Z

+                Url::parse("s3://bucket/prefix with space/").unwrap(),
+                "/prefix%20with%20space/",
+            ),
+            //(Url::parse("s3://bucket/special&chars/你好/😊").unwrap(), "/special&chars/你好/😊/"),


should this work, or should we remove it?

This is a hilariously invasive change insofar that we have URL-like strings leaking out all over the place in our codebase. This channge attempts to wrangle as much of that together as possible and has test API changes which support the preceeding commit where _uri functions become _url Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

The URLs used when writing entries into the table should always be normalized to ensure that the value of table_url() is identical to the one that is being written into the DynamoDB table. There may be some more testing required with the Delta/Spark implementation of S3DynamoDbLogStore to ensure that they are always generating URLs to the table with trailing slashes. Delta/Spark stores paths as [org.apache.hadoop.fs.Path](https://hadoop.apache.org/docs/stable/api/org/apache/hadoop/fs/Path.html) which should have URL-like semantics. Without treating trailing slashes as required, this could lead to inconsistencies between the two implementations. Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

Any places where we see hard-coded string formatting (e.g. format!()) around Url is likely to be **wrong** and need to be removed. This commit fixes the failing integration tests with the lakefs connector Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

…ad-bearing For better or worse, the trailing slashes on the `tablePath` DynamoDB items are semantically important and a trailing slash coming from deltalake-core causes lookup failures with multiple writers between Spark and Rust. This change ensures that the S3DynamoDbLogStore is always removing the trailing slash should it exist. Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

ion-elgreco · 2025-12-14T16:55:25Z

 }

 impl LogStoreConfig {
+    pub fn new(mut location: Url, options: StorageConfig) -> Self {
+        if !location.path().ends_with('/') {


Why don't we use normalize_table_url here?

this is probably one of the many ones that was already sprinkled through the code. but would be good to update as well.

hntd187

I made it. LGTM, I left some nits, but approved whether you fix them or not.

hntd187 · 2025-12-14T16:07:36Z

@@ -261,7 +265,7 @@ async fn test_abort_commit_entry() -> TestResult<()> {
        .await?;

    // The entry should have been aborted - the latest entry should be one version lower
-    if let Some(new_entry) = client.get_latest_entry(&table.table_uri()).await? {
+    if let Some(new_entry) = client.get_latest_entry(&table.table_url().as_str()).await? {


Just the pattern is gonna be to use .as_str() and such?

hntd187 · 2025-12-14T16:07:58Z

@@ -336,7 +344,7 @@ async fn test_concurrent_writers() -> TestResult<()> {
    println!(">>> preparing table");
    let table = prepare_table(&context, "concurrent_writes").await?;
    println!(">>> table prepared");
-    let table_uri = table.table_uri();
+    let table_uri = table.table_url();


URI or URL?

hntd187 · 2025-12-14T16:10:27Z

@@ -72,7 +72,7 @@ async fn test_object_store_onelake_abfs() -> TestResult {
 #[allow(dead_code)]
 async fn read_write_test_onelake(context: &IntegrationContext, path: &Path) -> TestResult {
    let table_uri = Url::parse(&context.root_uri()).unwrap();


Uri or URL?

hntd187 · 2025-12-14T16:14:22Z

+        let delta_scan = DeltaScan::new(
+            &wire.table_url,
+            wire.config,
+            (*inputs)[0].clone(),


This line totally makes sense, but I know we can't do much here.

hntd187 · 2025-12-14T16:16:25Z

+    }
+}
+
+#[non_exhaustive]


What does this mean here?

hntd187 · 2025-12-14T16:39:41Z

@@ -421,7 +416,7 @@ impl std::future::IntoFuture for WriteBuilder {

    fn into_future(self) -> Self::IntoFuture {
        let this = self;
-        let table_uri = this.log_store.root_uri();
+        let table_uri = this.log_store.root_url().clone();


Uri or URL?

hntd187 · 2025-12-14T16:42:48Z

                DeltaTable::new(log_store, Default::default()),
            )
        } else {
            let storage_url =
                ensure_table_uri(self.location.clone().ok_or(CreateError::MissingLocation)?)?;
            (
-                storage_url.as_str().to_string(),
-                DeltaTableBuilder::from_uri(storage_url)?
+                storage_url.clone(),


never thought I'd rather see a clone than what it used to be

hntd187 · 2025-12-14T16:49:44Z

-    let trimmed_path = url.path().trim_end_matches('/').to_owned();
-    url.set_path(&trimmed_path);
-    Ok(url)
+    // We should always be normalizing the table URL because trailing or redundant slashes can be


glad to see this go away

hntd187 · 2025-12-14T16:52:12Z

@@ -140,45 +137,6 @@ impl DeltaTableState {
        self.snapshot.snapshot().tombstones(log_store)
    }

-    /// Full list of add actions representing all parquet files that are part of the current


ion-elgreco

lgtm ;)

…e references This is a follow up to delta-io#3962 where some leftover comments were not addressed Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

…e references This is a follow up to #3962 where some leftover comments were not addressed Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

…e references This is a follow up to delta-io#3962 where some leftover comments were not addressed Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

github-project-automation bot added this to delta-rust Dec 2, 2025

github-actions bot added binding/python Issues for the Python package binding/rust Issues for the Rust crate delta-inspect labels Dec 2, 2025

rtyler force-pushed the next-version-three-zero branch 2 times, most recently from 8c9e6e6 to b5e4cd0 Compare December 2, 2025 14:49

roeap reviewed Dec 2, 2025

View reviewed changes

Comment thread crates/core/src/table/mod.rs

rtyler force-pushed the next-version-three-zero branch 2 times, most recently from 059834e to 659b756 Compare December 3, 2025 15:59

rtyler force-pushed the next-version-three-zero branch from 659b756 to 6740f2f Compare December 5, 2025 15:26

github-actions bot added the documentation Improvements or additions to documentation label Dec 5, 2025

rtyler force-pushed the next-version-three-zero branch 3 times, most recently from 07e5913 to 30dc809 Compare December 12, 2025 17:08

rtyler added 9 commits December 14, 2025 13:18

chore: bumping minor versions across the crates

1ae4594

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: cleaning up calls to deprecated file path functions

5d49d93

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: remove delta-inspect and its maintenance

0b2ba54

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: recactor LogStoreConfig for private members and clean up

99b4955

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: upgrade to rust edition 2024

8ef9cc2

This is almost entirely formatting changes, which is a _wee_ bit annoying. There were no practical code changes required however. Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: use the tempdir crate in a predictable way for set_tbl_properties

e7f5099

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

chore: enable python tests to pass again

0eb7fd7

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

rtyler force-pushed the next-version-three-zero branch from 30dc809 to df3739a Compare December 14, 2025 13:51

rtyler marked this pull request as ready for review December 14, 2025 15:42

rtyler requested a review from ion-elgreco as a code owner December 14, 2025 15:42

rtyler marked this pull request as draft December 14, 2025 15:44

rtyler marked this pull request as ready for review December 14, 2025 15:53

rtyler enabled auto-merge (rebase) December 14, 2025 15:53

rtyler force-pushed the next-version-three-zero branch 2 times, most recently from ee617c8 to da5c1cc Compare December 14, 2025 16:24

rtyler force-pushed the next-version-three-zero branch from da5c1cc to d107d57 Compare December 14, 2025 16:33

ion-elgreco reviewed Dec 14, 2025

View reviewed changes

roeap previously approved these changes Dec 14, 2025

View reviewed changes

rtyler dismissed roeap’s stale review via e64c8e1 December 14, 2025 16:53

rtyler added 4 commits December 14, 2025 16:53

rtyler force-pushed the next-version-three-zero branch from e64c8e1 to 80ab73b Compare December 14, 2025 16:53

ion-elgreco reviewed Dec 14, 2025

View reviewed changes

hntd187 approved these changes Dec 14, 2025

View reviewed changes

rtyler merged commit 2748585 into delta-io:main Dec 14, 2025
41 of 45 checks passed

github-project-automation bot moved this to Done in delta-rust Dec 14, 2025

ion-elgreco approved these changes Dec 14, 2025

View reviewed changes

rtyler mentioned this pull request Dec 14, 2025

chore: normalize Url going into logstore and update everything to take references #3985

Merged

rtyler mentioned this pull request Dec 16, 2025

feat: support full path URIs #3963

Draft

rtyler added a commit that referenced this pull request Dec 16, 2025

chore: normalize Url going into logstore and update everything to tak…

65dabb9

…e references This is a follow up to #3962 where some leftover comments were not addressed Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>

johnnyg mentioned this pull request Jan 26, 2026

[Bug]: create_write_transaction no longer overwrites partitions #4126

Closed

		// NOTE: the lack of trailing slashes is a load-bearing implementation
		// detail between the Delta/Spark and delta-rs S3DynamoDbLogStore

Conversation

rtyler commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

roeap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

corwinjoy commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roeap left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hntd187 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ion-elgreco left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rtyler commented Dec 2, 2025 •

edited

Loading

codecov bot commented Dec 2, 2025 •

edited

Loading