HonahX merged PR #288:
URL: https://github.com/apache/iceberg-python/pull/288
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1907487756
> Overall LGTM! Thank you very much @mgmarino! Just adding one final request
to make `IcebergSchemaToGlueType` internal by adding `_`. Sorry I missed that
early
Sure, np!
HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1464346417
##
pyiceberg/catalog/glue.py:
##
@@ -84,17 +110,97 @@ def _construct_parameters(
return new_parameters
+GLUE_PRIMITIVE_TYPES = {
+BooleanType: "boolean
HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1464342275
##
tests/catalog/integration_test_glue.py:
##
@@ -279,6 +379,20 @@ def test_commit_table_update_schema(
assert test_catalog._parse_metadata_version(table.metad
nicor88 commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1905753459
@HonahX @mgmarino all good on my side, all worked with a new table and with
the last commit 💯
great work @mgmarino !
--
This is an automated message from the Apache Git Service
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462796650
##
pyiceberg/catalog/glue.py:
##
@@ -84,17 +110,97 @@ def _construct_parameters(
return new_parameters
+GLUE_PRIMITIVE_TYPES = {
+BooleanType: "boole
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1905409366
> @nicor88. I tried the same code you posted along with this PR, and it
seemed to work on my side. Each time I ran the code, a new "Alice" and a new
"Bob" were appended to the
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462802332
##
tests/catalog/integration_test_glue.py:
##
@@ -279,6 +379,20 @@ def test_commit_table_update_schema(
assert test_catalog._parse_metadata_version(table.met
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462796650
##
pyiceberg/catalog/glue.py:
##
@@ -84,17 +110,97 @@ def _construct_parameters(
return new_parameters
+GLUE_PRIMITIVE_TYPES = {
+BooleanType: "boole
HonahX commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1905350230
> @mgmarino @HonahX - I was testing this, and after the change I confirm
that I can query the table in Athena (I'm still doing some deep dive on why the
table is not droppable in athe
HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1462684106
##
tests/catalog/integration_test_glue.py:
##
@@ -279,6 +379,20 @@ def test_commit_table_update_schema(
assert test_catalog._parse_metadata_version(table.metad
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903703045
Integration tests added in 40ab6e617c96712d5020e06b741fdbbd1963e75a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub
nicor88 commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903624202
@mgmarino @HonahX - I was testing this, and after the change I confirm that
I can query the table in Athena (I'm still doing some deep dive on why the
table is not droppable in athen
HonahX commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903540111
@mgmarino You are correct. We need to use our own AWS accounts to run them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903532176
@HonahX I just realized that the integration tests for glue are not
automatically collected/run in CI, so I guess these are just up to "us" to run
by hand? That makes it a bit easie
HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1461512416
##
pyiceberg/catalog/glue.py:
##
@@ -84,19 +110,105 @@ def _construct_parameters(
return new_parameters
+def _type_to_glue_type_string(input_type: IcebergT
nicor88 commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903491270
Regarding moto - I totally agree with you @mgmarino, it helps to test only
API specs, and as the main catalog source of true is glue (also for athena),
mocking athena via moto is not
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1903463726
Thanks, @HonahX, @nicor88.
Yes, I verified the correct behavior as well (creation of table, schema
change, etc.) before pushing, but I am happy to formalize this in an
integr
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1461475758
##
pyiceberg/catalog/glue.py:
##
@@ -84,19 +110,105 @@ def _construct_parameters(
return new_parameters
+def _type_to_glue_type_string(input_type: Iceber
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1461474013
##
pyiceberg/catalog/glue.py:
##
@@ -84,19 +110,105 @@ def _construct_parameters(
return new_parameters
+def _type_to_glue_type_string(input_type: Iceber
mgmarino commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1461464809
##
pyiceberg/catalog/glue.py:
##
@@ -84,19 +110,105 @@ def _construct_parameters(
return new_parameters
+def _type_to_glue_type_string(input_type: Iceber
HonahX commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1902879704
@mgmarino @nicor88 Thanks for your input.
> Did you try to evolve the table schema and see if the changes are properly
updated in glue and usable in Athena?
I did a simp
nicor88 commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1902724675
Thanks @mgmarino/@HonahX something else that comes to mind when working with
glue/ Athena (valid for other engines too).
Did you try to evolve the table schema and see if the chang
mgmarino commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1902635769
> Also, if possible, could you please also add some tests in
[integration_test_glue.py](https://github.com/apache/iceberg-python/blob/3085c404c99ba8c5c8856f21a6c8d63a12ca0113/tests/c
HonahX commented on PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#issuecomment-1902533747
Also, if possible, could you please also add some tests in
[integration_test_glue.py](https://github.com/apache/iceberg-python/blob/3085c404c99ba8c5c8856f21a6c8d63a12ca0113/tests/catal
HonahX commented on code in PR #288:
URL: https://github.com/apache/iceberg-python/pull/288#discussion_r1460723261
##
pyiceberg/catalog/glue.py:
##
@@ -84,19 +110,105 @@ def _construct_parameters(
return new_parameters
+def _type_to_glue_type_string(input_type: IcebergT
mgmarino opened a new pull request, #288:
URL: https://github.com/apache/iceberg-python/pull/288
Resolves #216.
This PR adds information about the schema (on update/create) and
location (create) of the table to Glue, enabling both an improved UI
experience as well as querying wit
27 matches
Mail list logo
