Skip to content

Commit 6a23b81

Browse files
committed
revert(datasets): remove changes unrelated to mode
Signed-off-by: Deepyaman Datta <[email protected]>
1 parent d922af1 commit 6a23b81

File tree

4 files changed

+3
-212
lines changed

4 files changed

+3
-212
lines changed

kedro-datasets/RELEASE.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
| `polars.PolarsDatabaseDataset` | A dataset to load and save data to a SQL backend using Polars | `kedro_datasets_experimental.polars` |
1010

1111
- `ibis.TableDataset`: Added configurable save modes via `save_args.mode`, supporting "append", "overwrite", "error"/"errorifexists", and "ignore". Legacy `save_args.overwrite` is mapped to `mode` for backward compatibility; specifying both is now an error.
12-
- `ibis.TableDataset`: Added a `credentials` parameter (string URI or dict, optionally with `con`) that supersedes the `connection` parameter. If both are provided, `credentials` takes precedence and a deprecation warning is issued.
1312

1413
## Bug fixes and other changes
1514

kedro-datasets/kedro_datasets/ibis/table_dataset.py

Lines changed: 2 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ def __init__( # noqa: PLR0913
8585
*,
8686
table_name: str,
8787
database: str | None = None,
88-
credentials: dict[str, Any] | str | None = None,
8988
connection: dict[str, Any] | None = None,
9089
load_args: dict[str, Any] | None = None,
9190
save_args: dict[str, Any] | None = None,
@@ -117,10 +116,6 @@ def __init__( # noqa: PLR0913
117116
in a multi-level table hierarchy.
118117
connection: Configuration for connecting to an Ibis backend.
119118
If not provided, connect to DuckDB in in-memory mode.
120-
credentials: Connection information (e.g.
121-
user, password, token, account). If provided, these values
122-
overlay the base `connection` configuration. May also be a
123-
connection string.
124119
load_args: Additional arguments passed to the Ibis backend's
125120
`read_{file_format}` method.
126121
save_args: Additional arguments passed to the Ibis backend's
@@ -139,32 +134,7 @@ def __init__( # noqa: PLR0913
139134

140135
self._table_name = table_name
141136
self._database = database
142-
self._credentials = deepcopy(credentials) or {}
143-
if self._credentials != {}:
144-
self._backend_name = self._get_backend_name()
145-
146-
# Prefer credentials if provided, else fallback to connection.
147-
self._connection_config = connection or (
148-
self.DEFAULT_CONNECTION_CONFIG if self._credentials == {} else {}
149-
)
150-
if self._credentials is not None:
151-
if isinstance(self._credentials, str):
152-
self._connection_config.update(
153-
{
154-
"backend": self._backend_name,
155-
"con": self._credentials,
156-
}
157-
)
158-
elif (
159-
isinstance(self._credentials, dict)
160-
and "backend" not in self._credentials
161-
and "con" in self._credentials
162-
):
163-
self._connection_config.update(
164-
self._credentials | {"backend": self._backend_name}
165-
)
166-
else:
167-
self._connection_config.update(self._credentials)
137+
self._connection_config = connection or self.DEFAULT_CONNECTION_CONFIG
168138
self.metadata = metadata
169139

170140
# Set load and save arguments, overwriting defaults if provided.
@@ -201,10 +171,6 @@ def _connect(self) -> BaseBackend:
201171
import ibis # noqa: PLC0415
202172

203173
config = deepcopy(self._connection_config)
204-
# If credentials is a dict with a 'con' key, treat as connection string
205-
if isinstance(config, dict) and "con" in config:
206-
return ibis.connect(config.pop("con"))
207-
# Otherwise, treat as expanded dict (params)
208174
backend = getattr(ibis, config.pop("backend"))
209175
return backend.connect(**config)
210176

@@ -216,12 +182,7 @@ def connection(self) -> BaseBackend:
216182
def load(self) -> ir.Table:
217183
return self.connection.table(self._table_name, **self._load_args)
218184

219-
# Users should wrap their DataFrame-like object in an ibis.memtable for in-memory data.
220-
# https://github.com/ibis-project/ibis/blob/df4f1858e7f9bc36d589dcf2a53f9f16b1acd92a/ibis/backends/duckdb/__init__.py#L180
221185
def save(self, data: ir.Table) -> None:
222-
# treat empty ir.Table as a no-op
223-
if isinstance(data, ir.Table) and data.count().execute() == 0:
224-
return
225186
writer = getattr(self.connection, f"create_{self._materialized}")
226187
if self._mode == "append":
227188
if not self._exists():
@@ -243,22 +204,6 @@ def save(self, data: ir.Table) -> None:
243204
return
244205
writer(self._table_name, data, overwrite=False, **self._save_args)
245206

246-
def _get_backend_name(self) -> str | None:
247-
"""Get the backend name from the connection config or connection string."""
248-
config = self._credentials
249-
# If dict and has 'backend'
250-
if isinstance(config, dict) and "backend" in config:
251-
return config["backend"]
252-
# If string, parse as backend://...
253-
if isinstance(config, str):
254-
if "://" in config:
255-
return config.split("://", 1)[0]
256-
# If dict with 'con' key
257-
if isinstance(config, dict) and "con" in config:
258-
con_str = config["con"]
259-
if "://" in con_str:
260-
return con_str.split("://", 1)[0]
261-
262207
def _describe(self) -> dict[str, Any]:
263208
load_args = deepcopy(self._load_args)
264209
save_args = deepcopy(self._save_args)
@@ -267,7 +212,7 @@ def _describe(self) -> dict[str, Any]:
267212
return {
268213
"table_name": self._table_name,
269214
"database": self._database,
270-
"backend": self.connection.name,
215+
"backend": self._connection_config["backend"],
271216
"load_args": load_args,
272217
"save_args": save_args,
273218
"materialized": self._materialized,

kedro-datasets/tests/ibis/conftest.py

Lines changed: 0 additions & 29 deletions
This file was deleted.

kedro-datasets/tests/ibis/test_table_dataset.py

Lines changed: 1 addition & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -37,39 +37,17 @@ def connection_config(request, database):
3737
)
3838

3939

40-
@pytest.fixture(params=[_SENTINEL])
41-
def credentials_config(request, database):
42-
return (
43-
None
44-
if request.param is _SENTINEL # `None` is a valid value to test
45-
else request.param
46-
)
47-
48-
4940
@pytest.fixture(params=[_SENTINEL])
5041
def table_name(request):
5142
# Default table name when not explicitly parametrized
5243
return "test" if request.param is _SENTINEL else request.param
5344

5445

55-
# @pytest.fixture(params=['test'])
56-
# def table_name(request):
57-
# return request.param
58-
59-
6046
@pytest.fixture
61-
def table_dataset(
62-
database_name,
63-
credentials_config,
64-
connection_config,
65-
load_args,
66-
save_args,
67-
table_name,
68-
):
47+
def table_dataset(table_name, database_name, connection_config, load_args, save_args):
6948
return TableDataset(
7049
table_name=table_name,
7150
database=database_name,
72-
credentials=credentials_config,
7351
connection=connection_config,
7452
load_args=load_args,
7553
save_args=save_args,
@@ -260,17 +238,6 @@ def test_describe_includes_backend_mode_and_materialized(self, table_dataset):
260238
assert "database" not in desc["load_args"]
261239
assert "database" not in desc["save_args"]
262240

263-
@pytest.mark.parametrize(
264-
"save_args",
265-
[{"materialized": "table"}],
266-
indirect=True,
267-
)
268-
def test_save_empty_dataframe_is_noop(self, table_dataset):
269-
"""Saving an empty DataFrame should be a no-op (no table created)."""
270-
empty_table = ibis.memtable(pd.DataFrame({"col1": [], "col2": [], "col3": []}))
271-
table_dataset.save(empty_table)
272-
assert not table_dataset.exists()
273-
274241
@pytest.mark.parametrize("load_args", [{"database": "test"}], indirect=True)
275242
def test_load_extra_params(self, table_dataset, load_args):
276243
"""Test overriding the default load arguments."""
@@ -378,100 +345,9 @@ def test_connection_config(self, mocker, table_dataset, connection_config, key):
378345
table_dataset.load()
379346
assert ("ibis", key) in table_dataset._connections
380347

381-
@pytest.mark.parametrize(
382-
("credentials_config", "key"),
383-
[
384-
(
385-
"postgres://xxxxxx.postgres.database.azure.com:5432/postgres",
386-
(
387-
("backend", "postgres"),
388-
(
389-
"con",
390-
"postgres://xxxxxx.postgres.database.azure.com:5432/postgres",
391-
),
392-
),
393-
),
394-
(
395-
{
396-
"con": "postgres://[email protected]:5432/postgres"
397-
},
398-
(
399-
("backend", "postgres"),
400-
(
401-
"con",
402-
"postgres://[email protected]:5432/postgres",
403-
),
404-
),
405-
),
406-
(
407-
{
408-
"backend": "postgres",
409-
"database": "postgres",
410-
"host": "xxxx.postgres.database.azure.com",
411-
},
412-
(
413-
("backend", "postgres"),
414-
("database", "postgres"),
415-
("host", "xxxx.postgres.database.azure.com"),
416-
),
417-
),
418-
],
419-
)
420-
@pytest.mark.parametrize("connection_config", [None], indirect=True)
421-
def test_connection_config_with_credentials(
422-
self, mocker, table_dataset, credentials_config, key
423-
):
424-
# 1) isolate the cache so parametrized cases don't reuse connections
425-
426-
if isinstance(credentials_config, str) or "backend" not in credentials_config:
427-
backend = "postgres"
428-
else:
429-
backend = credentials_config["backend"]
430-
431-
mocker.patch(f"ibis.{backend}")
432-
conn = table_dataset.connection
433-
assert conn is not None
434-
table_dataset.load()
435-
assert ("ibis", key) in table_dataset._connections
436-
437348
def test_save_data_loaded_using_file_dataset(self, file_dataset, table_dataset):
438349
"""Test interoperability of Ibis datasets sharing a database."""
439350
dummy_table = file_dataset.load()
440351
assert not table_dataset.exists()
441352
table_dataset.save(dummy_table)
442353
assert table_dataset.exists()
443-
444-
# Additional tests for _get_backend_name branch coverage
445-
class TestGetBackendName:
446-
def test_get_backend_name_dict_with_backend(self):
447-
ds = TableDataset(table_name="t")
448-
ds._credentials = {"backend": "postgres", "database": "db"}
449-
assert ds._get_backend_name() == "postgres"
450-
451-
def test_get_backend_name_dict_without_backend_or_con(self):
452-
ds = TableDataset(table_name="t")
453-
ds._credentials = {"user": "u", "password": "p"}
454-
assert ds._get_backend_name() is None
455-
456-
def test_get_backend_name_string_with_scheme(self):
457-
ds = TableDataset(table_name="t")
458-
ds._credentials = "mysql://xxxxxx@host:3306/dbname"
459-
assert ds._get_backend_name() == "mysql"
460-
461-
def test_get_backend_name_string_without_scheme(self):
462-
ds = TableDataset(table_name="t")
463-
ds._credentials = "not_a_url_string"
464-
assert ds._get_backend_name() is None
465-
466-
def test_get_backend_name_dict_with_con_and_scheme(self):
467-
ds = TableDataset(table_name="t")
468-
ds._credentials = {
469-
"con": "postgres://xxxxxx@host:5432/dbname",
470-
"some_other": "value",
471-
}
472-
assert ds._get_backend_name() == "postgres"
473-
474-
def test_get_backend_name_dict_with_con_without_scheme(self):
475-
ds = TableDataset(table_name="t")
476-
ds._credentials = {"con": "sqlite_memory"}
477-
assert ds._get_backend_name() is None

0 commit comments

Comments
 (0)