fix: creating new DeltaTable with invalid table name path no longer creates empty directory

[smeyerre]

Description

Previously when using the Python bindings, creating a DeltaTable with an incorrect table path caused an empty directory to be made. Now no directory is created and an appropriate error message is served:

>>> from deltalake import DeltaTable
>>> dt = DeltaTable('nonexistent_table')
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    dt = DeltaTable('nonexistent_table')
  File "/home/sam/github/delta-rs/python/deltalake/table.py", line 174, in __init__
    self._table = RawDeltaTable(
                  ~~~~~~~~~~~~~^
        str(table_uri),
        ^^^^^^^^^^^^^^^
    ...<3 lines>...
        log_buffer_size=log_buffer_size,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
_internal.TableNotFoundError: Local path "nonexistent_table" does not exist or you don't have access!

Related Issue(s)

• closes Opening an incorrect table path causes an empty directory to be made #3060

[github-actions]

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

[smeyerre]

This is likely out of scope for this PR, but I'm seeing other places (eg. in is_deltatable) where we're currently using from_url where it would probably make more sense to use from_valid_url as well. In fact I'm curious, when would we want to ever use from_url instead?

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 74.11%. Comparing base (2f990fe) to head (b78dee6).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
python/src/lib.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3504      +/-   ##
==========================================
+ Coverage   74.02%   74.11%   +0.08%     
==========================================
  Files         148      150       +2     
  Lines       44335    44551     +216     
  Branches    44335    44551     +216     
==========================================
+ Hits        32821    33017     +196     
- Misses       9380     9381       +1     
- Partials     2134     2153      +19     

☔ 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.

[ion-elgreco]

@smeyerre can you add a test as well?

[roeap]

I think we do need to be a little bit careful here. Valid url not only means that the directory exists, but we also ensure e.g. that a path ends in '/' which is quite important since e.g. Url::join behaves quite differently if it does not.

We also need to validate how the create command behaves in this case. We still use a prefixed store in many places and while object store will create any directory within its scope, it might not create any parent directory that is part of the prefix. Since we can no longer guarantee that a directory exists we now need to ensure that at the very least in the CreateOperation and likely also in some other places (maybe convert_to_delta)?

All that said, I do agree that we should not just eagerly create the directory, before we know its what the user wants.

[smeyerre]

I think we do need to be a little bit careful here. Valid url not only means that the directory exists, but we also ensure e.g. that a path ends in '/' which is quite important since e.g. Url::join behaves quite differently if it does not.

We also need to validate how the create command behaves in this case. We still use a prefixed store in many places and while object store will create any directory within its scope, it might not create any parent directory that is part of the prefix. Since we can no longer guarantee that a directory exists we now need to ensure that at the very least in the CreateOperation and likely also in some other places (maybe convert_to_delta)?

All that said, I do agree that we should not just eagerly create the directory, before we know its what the user wants.

Just want to make sure I'm understanding fully since I'm not super experienced with DeltaTable. You're saying we want to be extra sure that things like CreateBuilder, ConvertToDeltaBuilder, etc still create the directory properly? And that table paths are formatted with trailing slashes after passing through from_valid_url?

Could you give me an example of the expected behavior you're describing with object store directory creation as well?

Thanks!