feat: implement InMemoryCatalog as a subclass of SqlCatalog #1140
Conversation
tests/catalog/test_base.py
Outdated
| @@ -62,21 +50,20 @@ | |||
| DEFAULT_WAREHOUSE_LOCATION = "file:///tmp/warehouse" | |||
|
|
|||
|
|
|||
| class InMemoryCatalog(MetastoreCatalog): | |||
| class InMemoryCatalog(SqlCatalog): | |||
There was a problem hiding this comment.
nit, take this out of tests/
tests/catalog/test_base.py
Outdated
| namespace_identifier = Catalog.namespace_from(identifier) | ||
| if not self._namespace_exists(namespace_identifier): | ||
| self.create_namespace(namespace_identifier) |
There was a problem hiding this comment.
nit: instead of changing the behavior of the function, let's fix the tests
hussein-awala
force-pushed
the
remove_InMemoryCatalog
branch
from
21d6d2f
to
aa6efc6
Compare
hussein-awala
changed the title
hussein-awala
force-pushed
the
remove_InMemoryCatalog
branch
from
aa6efc6
to
6089ca2
Compare
hussein-awala
force-pushed
the
remove_InMemoryCatalog
branch
from
6089ca2
to
b6af81a
Compare
|
@kevinjqliu I applied what you suggested in the comment above, could you recheck it now? |
| This is useful for test, demo, and playground but not in production as data is not persisted. | ||
| """ | ||
|
|
||
| def __init__(self, name: str, warehouse: str = "file:///tmp/warehouse", **kwargs: str) -> None: |
There was a problem hiding this comment.
nit: let's use something like /tmp/iceberg/warehouse to not conflict with other tmp directories. Also I'm not sure if this works when the warehouse directory is not created yet.
There was a problem hiding this comment.
nit: i'd like to keep this as test_base because I want to parameterize all tests to make sure all the catalogs have the same behaviors (see #813)
| DROP_NOT_EXISTING_NAMESPACE_ERROR = "Namespace does not exist: \\('com', 'organization', 'department'\\)" | ||
| NO_SUCH_NAMESPACE_ERROR = "Namespace com.organization.department does not exists" |
| @@ -806,7 +831,7 @@ def test_json_properties_get_table_does_not_exist(catalog: InMemoryCatalog) -> N | |||
| runner = CliRunner() | |||
| result = runner.invoke(run, ["--output=json", "properties", "get", "table", "doesnotexist"]) | |||
| assert result.exit_code == 1 | |||
| assert result.output == """{"type": "NoSuchTableError", "message": "Table does not exist: ('doesnotexist',)"}\n""" | |||
| assert result.output == """{"type": "ValueError", "message": "Empty namespace identifier"}\n""" | |||
There was a problem hiding this comment.
nit: shouldnt this be NoSuchTableError? maybe the namespace needs to be created first
|
@Fokko wydt of this change? i remember we had past discussions on adding a "new" catalog implementation |
closes: #1110
This PR implement a new catalog
InMemoryCatalogas a subclass ofSqlCatalogwith SQLite in-memory.