sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2554801147
@kevinjqliu and @Fokko - thank you for your reviews!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL
sungwy merged PR #963:
URL: https://github.com/apache/iceberg-python/pull/963
--
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
Fokko commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1892453681
##
pyiceberg/table/__init__.py:
##
@@ -132,7 +132,8 @@
)
from pyiceberg.utils.concurrent import ExecutorFactory
from pyiceberg.utils.config import Config
-from py
sungwy commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1891936098
##
pyiceberg/table/__init__.py:
##
@@ -132,7 +132,8 @@
)
from pyiceberg.utils.concurrent import ExecutorFactory
from pyiceberg.utils.config import Config
-from p
Fokko commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1891313375
##
pyiceberg/table/__init__.py:
##
@@ -132,7 +132,8 @@
)
from pyiceberg.utils.concurrent import ExecutorFactory
from pyiceberg.utils.config import Config
-from py
Fokko commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1891313171
##
pyiceberg/catalog/__init__.py:
##
@@ -70,7 +70,8 @@
RecursiveDict,
)
from pyiceberg.utils.config import Config, merge_config
-from pyiceberg.utils.deprecat
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2552557904
> And there is something very off with the commits, we want to fix that as
well, otherwise we'll get a commit with all the authors combined I'm afraid 😅
Yeah I've tried rebasing
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2552556489
> * identifier
Thanks for the sharp eye @kevinjqliu - I've removed these as well in the
newest commit
--
This is an automated message from the Apache Git Service.
To respond
sungwy commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1891001742
##
tests/catalog/test_base.py:
##
@@ -514,7 +514,7 @@ def test_rename_table(catalog: InMemoryCatalog) -> None:
# Then
assert table._identifier == Catalo
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2551547012
@Fokko and @kevinjqliu - Thank you both! I'll take a look at fixing the
commit history later today
--
This is an automated message from the Apache Git Service.
To respond to the mes
Fokko commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2551520152
And there is something very off with the commits, we want to fix that as
well, otherwise we'll get a commit with all the authors combined I'm afraid 😅
--
This is an automated messag
kevinjqliu commented on code in PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#discussion_r1889024364
##
tests/catalog/test_base.py:
##
@@ -514,7 +514,7 @@ def test_rename_table(catalog: InMemoryCatalog) -> None:
# Then
assert table._identifier == Ca
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2530031186
Hi @Fokko @kevinjqliu - could I ask for your review on this PR? I think
we'll be able to introduce this in 0.9.0 now that the deprecation notice has
been sent out :)
--
This is an
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2524829316
Now that the deprecation is out for 0.8.0 - I'll get working on this large
PR again
--
This is an automated message from the Apache Git Service.
To respond to the message, please lo
Fokko commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2316296889
While working on https://github.com/apache/iceberg-python/pull/1112, I fully
support this.
--
This is an automated message from the Apache Git Service.
To respond to the message, ple
kevinjqliu commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2273922765
So #994 will be included in the 0.8.0 release, which will still allow
catalog name but with a deprecation warning.
And this PR will be included in the 0.9.0 release and will fu
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2267201384
I opened up the mail list discussion:
https://lists.apache.org/thread/9zr19hxnbt3hg7lt55t6dfg6otv7zjz2
Unfortunately, there hasn't been a lot of community engagement on the mail
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2253118608
@corleyma and @c-thiel - @HonahX put together a fix for this release:
https://github.com/apache/iceberg-python/pull/964
Could we ask for your help in confirming if this resolves
kevinjqliu commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2251904029
+1 to remove the catalog name as part of the table identifier. #964 is a
good workaround to get into the 0.7.0 release.
This PR is great as a long-term plan to deprecate the
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2249980644
Hi @HonahX and @Fokko thank you for the detailed reviews! Given those
concerns, I think it would make sense to:
1. Introduce the fix @HonahX proposed to address #742 without introdu
HonahX commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2249883371
Thanks for raising the PR! I am also +1 on removing this. This seems
unnecessary and will inevitably lead to the issue as @sungwy mentioned in the
third point:
> identifier_to_
Fokko commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2249655208
Hey @sungwy thanks for raising this. Some historical context was added from
the beginning to copy some of the concepts of how it works in Java/Spark, but
to be frank, I don't think it
sungwy commented on PR #963:
URL: https://github.com/apache/iceberg-python/pull/963#issuecomment-2249217253
There is some more work to be done on this PR, including:
- deprecating `identifier_to_tuple_without_catalog` public method on the
catalog
- removing its usage across the board,
sungwy opened a new pull request, #963:
URL: https://github.com/apache/iceberg-python/pull/963
Currently, we optionally support catalog names being specified in the
identifier string. In other words, this means that we currently support
identifier names that look like `catalog_name.table_na
24 matches
Mail list logo
