Implementing namespace_exists function on the REST Catalog

[AhmedNader42]

As per #1430 I have added the namespace_exists functionality for REST Catalog.

Here's an example usage similar to the table_exists function of the same class

Please review the changes

[kevinjqliu]

nit: the tests here are only verifying against the mock response.
it would be great to test against the actual REST catalog in integration tests.

i noticed that does not exist right now, perhaps we can start a tests/integration/test_rest_catalog.py

[AhmedNader42]

Yes, Good point! I was looking for how to properly test this but didn't find any tests touching the REST Catalog. I thought it was a bit out of scope for the initial issue to add a new integration test flow.

Maybe we should open a new enhancement and add the details to it to implement a new integration test for REST Catalog at tests/integration/test_rest_catalog.py like you mentioned. Thoughts? I'm willing to pick it up

[sungwy]

Hi @AhmedNader42 - thank you very much for picking up this issue and getting a working solution up already!

I'm in agreement with @kevinjqliu 's comment, that it would be great to have an integration test as well, and I think it is well within the scope of this PR.

https://github.com/apache/iceberg-python/blob/main/tests/integration/test_reads.py already uses session_catalog in its tests, which is a rest catalog fixture defined in the conftest.py, and I think it would be a good place to add this test case.

[AhmedNader42]

Hello @sungwy, Thank you for pointing that out! Sounds good to me. I'll work on adding some test cases for REST Catalog in test_reads.py

Hi @AhmedNader42 - thank you very much for picking up this issue and getting a working solution up already!

I'm in agreement with @kevinjqliu 's comment, that it would be great to have an integration test as well, and I think it is well within the scope of this PR.

https://github.com/apache/iceberg-python/blob/main/tests/integration/test_reads.py already uses session_catalog in its tests, which is a rest catalog fixture defined in the conftest.py, and I think it would be a good place to add this test case.

[AhmedNader42]

I created a new file for REST Catalog integration tests tests/integration/test_rest_catalog.py as suggestsed by @kevinjqliu and included some tests covering the namespace_exists functionality. Kindly re-check

[kevinjqliu]

LGTM! Thank you for adding the integration test for rest catalog, much needed :)

I double-checked with namespaceExists in the REST spec, everything looks good.
Similar to tableExists, we're including 200 status code as a convenience. I'll see if we can add this to the spec itself

[kevinjqliu]

@AhmedNader42 looks like the RAT check failed. For new files, we need to include the ASF license on top, like

iceberg-python/tests/catalog/test_rest.py

Lines 1 to 16 in ceffe08

	# Licensed to the Apache Software Foundation (ASF) under one
	# or more contributor license agreements. See the NOTICE file
	# distributed with this work for additional information
	# regarding copyright ownership. The ASF licenses this file
	# to you under the Apache License, Version 2.0 (the
	# "License"); you may not use this file except in compliance
	# with the License. You may obtain a copy of the License at
	#
	# http://www.apache.org/licenses/LICENSE-2.0
	#
	# Unless required by applicable law or agreed to in writing,
	# software distributed under the License is distributed on an
	# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
	# KIND, either express or implied. See the License for the
	# specific language governing permissions and limitations
	# under the License.

[Fokko]

Hmm, I didn't know that spaces are allowed :D

[kevinjqliu]

Thanks for the contribution, @AhmedNader42 !
I created #1439 so that we can add more integration tests for the REST catalog.
I also started this devlist discussion to propose adding 200 status code as a valid response for HEAD requests.