You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "gabeiglio (via GitHub)" <gi...@apache.org> on 2025/08/30 01:48:49 UTC

[PR] Fix and refactor error handling in rest catalog [iceberg-python]

gabeiglio opened a new pull request, #2404:
URL: https://github.com/apache/iceberg-python/pull/2404

   <!--
   Thanks for opening a pull request!
   -->
   
   <!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
   <!-- Closes #${GITHUB_ISSUE_ID} -->
   
   # Rationale for this change
   
   Error handling was not following the rest specification in the following cases: 
   
   [TODO] Specify all cases where exceptions were incorrect or missing. (Also mention the recent changes for 5xx error cases to mark as non-retryable like 503)
   
   [TODO] Explain changes
   
   ## Are these changes tested?
   
   [TODO] Add unit and integration tests on test_catalog
   
   ## Are there any user-facing changes?
   
   <!-- In the case of user-facing changes, please add the changelog label. -->
   


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2404: Fix and refactor error handling in rest catalog
URL: https://github.com/apache/iceberg-python/pull/2404


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#issuecomment-3594488362

   Thanks @gabeiglio for working on this, and thanks @geruh for bringing this back up in my mailbox :) I agree with what Drew is saying, but in general, I think this is a great improvement 👍 Could you follow up, so we can get this in?


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "gabeiglio (via GitHub)" <gi...@apache.org>.
gabeiglio commented on code in PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#discussion_r2596226844


##########
tests/integration/test_deletes.py:
##########
@@ -43,7 +43,7 @@ def run_spark_commands(spark: SparkSession, sqls: list[str]) -> None:
 def test_table(session_catalog: RestCatalog) -> Generator[Table, None, None]:
     identifier = "default.__test_table"
     arrow_table = pa.Table.from_arrays([pa.array([1, 2, 3, 4, 5]), pa.array(["a", "b", "c", "d", "e"])], names=["idx", "value"])
-    test_table = session_catalog.create_table(
+    test_table = session_catalog.create_table_if_not_exists(

Review Comment:
   Not needed, rolled back!



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "geruh (via GitHub)" <gi...@apache.org>.
geruh commented on code in PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#discussion_r2596133013


##########
tests/integration/test_deletes.py:
##########
@@ -43,7 +43,7 @@ def run_spark_commands(spark: SparkSession, sqls: list[str]) -> None:
 def test_table(session_catalog: RestCatalog) -> Generator[Table, None, None]:
     identifier = "default.__test_table"
     arrow_table = pa.Table.from_arrays([pa.array([1, 2, 3, 4, 5]), pa.array(["a", "b", "c", "d", "e"])], names=["idx", "value"])
-    test_table = session_catalog.create_table(
+    test_table = session_catalog.create_table_if_not_exists(

Review Comment:
   Do we need this change? 



##########
tests/catalog/test_rest.py:
##########
@@ -1017,6 +1036,25 @@ def test_drop_table_404(rest_mock: Mocker) -> None:
     assert "Table does not exist" in str(e.value)
 
 
+def test_drop_table_404_non_existent_namespace(rest_mock: Mocker) -> None:
+    rest_mock.delete(
+        f"{TEST_URI}v1/namespaces/fokko/tables/does_not_exists",
+        json={
+            "error": {
+                "message": "Table does not exist: fokko.does_not_exists in warehouse 8bcb0838-50fc-472d-9ddb-8feb89ef5f1e",
+                "type": "NoSuchNamespaceErrorException",
+                "code": 404,
+            }
+        },
+        status_code=404,
+        request_headers=TEST_HEADERS,
+    )
+
+    with pytest.raises(NoSuchNamespaceError) as e:
+        RestCatalog("rest", uri=TEST_URI, token=TEST_TOKEN).drop_table(("fokko", "does_not_exists"))

Review Comment:
   lol



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#issuecomment-4130728751

   This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "jayceslesar (via GitHub)" <gi...@apache.org>.
jayceslesar commented on PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#issuecomment-3249001076

   more of a meta question but I wonder if we could auto generate some testing based off of the openapi spec upstream?


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] Fix and refactor error handling in rest catalog [iceberg-python]

Posted by "gabeiglio (via GitHub)" <gi...@apache.org>.
gabeiglio commented on PR #2404:
URL: https://github.com/apache/iceberg-python/pull/2404#issuecomment-3651117837

   @Fokko bringing this back up to your mailbox :)


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org