Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: harmonize and simplify storage configuration #1052

Merged
merged 29 commits into from
Jan 11, 2023
+469 −799
Merged

feat: harmonize and simplify storage configuration #1052

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
eb36d65
refactor: use new object_store config system
roeap Jan 5, 2023
338f459
build: try patching object store to see CI results
roeap Jan 5, 2023
6fa735f
fix: variant name
roeap Jan 5, 2023
9311eae
fix: clippy
roeap Jan 5, 2023
56d550c
chore: handle uncovered variants
roeap Jan 5, 2023
728863e
fix: update allow http config
roeap Jan 5, 2023
d07cb95
fix: try different test config
roeap Jan 5, 2023
35055ab
fix: add allow http opt
roeap Jan 5, 2023
045fa54
refactor: remove DeltaObjectStoreConfig
roeap Jan 7, 2023
823f9f8
refactor: move storage location to separate module
roeap Jan 7, 2023
b971efd
revert renaming env vars
roeap Jan 7, 2023
f076214
chore: format
roeap Jan 7, 2023
9f1484f
feat: handle some env vars
roeap Jan 7, 2023
1102adf
chore: clippy
roeap Jan 7, 2023
3def261
fix: cinfig with no features
roeap Jan 7, 2023
2ab7e55
fix: handle allow http
roeap Jan 7, 2023
31fe05b
fix: missing docs
roeap Jan 7, 2023
c3ef346
fix: detect allowed http when passed in config
roeap Jan 7, 2023
3646f2c
fix: set a default region in aws
roeap Jan 7, 2023
ae3e193
chore: clippy
roeap Jan 7, 2023
f706b92
chore: use released object_store
roeap Jan 9, 2023
60ac277
fix: set default region in test
roeap Jan 9, 2023
260ec27
fix: remove azure anon test
roeap Jan 9, 2023
9660e15
chore: use map reference
roeap Jan 9, 2023
04eaac1
fix: clippy
roeap Jan 9, 2023
780b04a
docs: add links to options / keys for storage backends
roeap Jan 9, 2023
c625240
Update rust/src/builder.rs
roeap Jan 10, 2023
4aeb972
Update python/docs/source/usage.rst
roeap Jan 10, 2023
95dc31b
Update rust/src/storage/config.rs
roeap Jan 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions python/tests/test_fs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def test_read_files(s3_localstack):
@pytest.mark.s3
@pytest.mark.integration
@pytest.mark.timeout(timeout=15, method="thread")
def test_s3_authenticated_read_write(s3_localstack_creds):
def test_s3_authenticated_read_write(s3_localstack_creds, monkeypatch):
monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously we were using the rusoto logic from our S3 storage options, which look in the envornment, then a profile file, and finally just defaults to us-east-1 as a region.

While slightly convenient, i felt this is a bit too much logic for us, and hoping, that the dynamo_lock crate will eventually also move to the official aws sdk, I also did not want to use the rusoto helper.

If this goes too much against the expectations of users, we can of course add some login for this.

The setting could of course also be passed with the storage options :)

# Create unauthenticated handler
storage_handler = DeltaStorageHandler(
"s3://deltars/",
Expand Down Expand Up @@ -184,13 +185,6 @@ def test_roundtrip_azure_env(azurite_env_vars, sample_data: pa.Table):
def test_roundtrip_azure_direct(azurite_creds, sample_data: pa.Table):
table_path = "az://deltars/roundtrip2"

# Fails without any creds
with pytest.raises(PyDeltaTableError):
anon_storage_options = {
key: value for key, value in azurite_creds.items() if "ACCOUNT" not in key
}
write_deltalake(table_path, sample_data, storage_options=anon_storage_options)
Comment on lines -187 to -192
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure what we did before, but now AZURE_STORAGE_USE_EMULATOR will pick up the default account / key if none are specified - so its hard to create a failing state with the emulator now.


# Can pass storage_options in directly
write_deltalake(table_path, sample_data, storage_options=azurite_creds)
dt = DeltaTable(table_path, storage_options=azurite_creds)
Expand Down
2 changes: 1 addition & 1 deletion rust/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ log = "0"
libc = ">=0.2.90, <1"
num-bigint = "0.4"
num-traits = "0.2.15"
object_store = "0.5.2"
object_store = "0.5.3"
once_cell = "1.16.0"
parking_lot = "0.12"
parquet = { version = "28", features = ["async"], optional = true }
Expand Down
Loading