Skip to content

Python: Add Hive Catalog #5391

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

Merged
rdblue merged 9 commits into from
Aug 2, 2022
Merged

Python: Add Hive Catalog #5391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
80f72a5
First version
Fokko Jul 28, 2022
d65a6dd
Working on testing
Fokko Jul 28, 2022
91f2d27
Merge branch 'master' of https://github.com/apache/iceberg into fd-ad…
Fokko Jul 31, 2022
a4ac613
Thanks for the comments
Fokko Jul 31, 2022
f6a5ad0
Merge branch 'master' of https://github.com/apache/iceberg into fd-ad…
Fokko Jul 31, 2022
1b656e5
Comments
Fokko Aug 2, 2022
2cb8905
Update hive.py
rdblue Aug 2, 2022
861c11d
Merge branch 'master' of https://github.com/apache/iceberg into fd-ad…
Fokko Aug 2, 2022
a5f8e7f
Fix conflicts
Fokko Aug 2, 2022
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
1 change: 1 addition & 0 deletions python/.pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# under the License.
---
files: ^python/
exclude: ^python/vendor/

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand Down
20 changes: 20 additions & 0 deletions python/LICENSE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,23 @@ Home page: https://avro.apache.org/
License: https://www.apache.org/licenses/LICENSE-2.0

--------------------------------------------------------------------------------

This product includes code from Apache Thrift.

* Uses the fb303.thrift file that's part of Hive's thrift service in vendor/fb303/

Copyright: 2006-2022 The Apache Software Foundation.
Home page: https://thrift.apache.org/
License: https://www.apache.org/licenses/LICENSE-2.0

--------------------------------------------------------------------------------

This product includes code from Apache Hive.

* Uses hive_metastore.thrift to generate the Hive Metastore client in vendor/hive_metastore/

Copyright: 2008-2022 The Apache Software Foundation.
Home page: https://hive.apache.org/
License: https://www.apache.org/licenses/LICENSE-2.0

--------------------------------------------------------------------------------
2 changes: 1 addition & 1 deletion python/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

install:
pip install poetry
poetry install -E pyarrow
poetry install -E pyarrow -E hive

lint:
poetry run pre-commit run --all-files
Expand Down
236 changes: 48 additions & 188 deletions python/poetry.lock
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions python/pyiceberg/catalog/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def create_table(
def load_table(self, identifier: str | Identifier) -> Table:
"""Loads the table's metadata and returns the table instance.

You can also use this method to check for table existence using 'try catalog.table() except TableNotFoundError'
You can also use this method to check for table existence using 'try catalog.table() except NoSuchTableError'
Note: This method doesn't scan data stored in the table.

Args:
Expand Down Expand Up @@ -166,7 +166,7 @@ def drop_namespace(self, namespace: str | Identifier) -> None:
"""

@abstractmethod
def list_tables(self, namespace: str | Identifier | None = None) -> list[Identifier]:
def list_tables(self, namespace: str | Identifier) -> list[Identifier]:
Copy link
Contributor

@rdblue rdblue Aug 1, 2022

Choose a reason for hiding this comment

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

I think that having None here was correct. You can list tables in the root namespace.

Copy link
Contributor Author

@Fokko Fokko Aug 2, 2022

Choose a reason for hiding this comment

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

Technically I understand it is possible, but this isn't the case for Hive and also not for the REST catalog (namespace is required): https://github.com/apache/iceberg/blob/master/open-api/rest-catalog-open-api.yaml#L401

Therefore I decided to remove it for now. WDYT?

Copy link
Contributor

@rdblue rdblue Aug 2, 2022

Choose a reason for hiding this comment

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

This isn't a change I would mix in with the current PR, and I probably wouldn't bother making it anyway. The Catalog contract does should allow this, so I don't see a reason to remove it from the contract documentation just because we don't have an implementation in the Python code yet.

"""List tables under the given namespace in the catalog.

If namespace not provided, will list all tables in the catalog.
Expand Down
Loading