feat: add load_view to REST catalog

[spr0els]

Rationale for this change

This is part of #818 and implements the load_view method for REST catalogs.

Are these changes tested?

Unit tests are added for:

• successful loading of a view
• thrown 404 error when trying to load a non existing view

Are there any user-facing changes?

This adds the load_view method for REST catalogs

[gabeiglio]

Nit: I would say we remove these.

[geruh]

+1 this is simply just load a view it doesn't have to follow the convention of load_table.

ref: https://github.com/apache/iceberg/blob/53374bde66955885de6fca0ea59b946346ec2d89/api/src/main/java/org/apache/iceberg/catalog/ViewCatalog.java#L49

[spr0els]

Updated the docstring

[gabeiglio]

Would also add a test that should return a view does not exists when trying to load a table with loadView

[spr0els]

Correct me if I'm wrong, but isn't this case already covered with the test_load_view_404 test?
If no view with the same identifier as the table identifier is found in the warehouse, this would be the same case as loading a non existent view.

[gabeiglio]

This is implementation dependent on the server, so this is an edge case to return the correct exception

But the test is better placed in integration tests rather than mocking

Would be cool to also add integration tests to the test_catalog.py

[geruh]

+1 this is simply just load a view it doesn't have to follow the convention of load_table.

ref: https://github.com/apache/iceberg/blob/53374bde66955885de6fca0ea59b946346ec2d89/api/src/main/java/org/apache/iceberg/catalog/ViewCatalog.java#L49

[gabeiglio]

Apart from adding integration tests LGTM

[geruh]

I think we just need a rebase here, and I agree with @gabeiglio on adding an integ test if tck has the view support.

[spr0els]

I added integration tests to tests/integration/test_rest_catalog.py because most catalogs don't have the methods for view interaction implemented yet. Putting the tests in test_catalog.py would cause too much error handling for most catalogs at the moment imho.

[rambleraptor]

I'm late to the party on this one, but this looks fantastic. Thanks for helping to push view support along!

[ebyhr]

Why is this NoSuchNamespaceErrorException instead of NoSuchViewException?

https://github.com/apache/iceberg/blob/f29a182eccc9287764a511767a79f741eb4135af/open-api/rest-catalog-open-api.yaml#L1648-L1657

404:
description:
Not Found - NoSuchViewException, view to load does not existcontent:
application/json:
schema:
$ref: '#/components/schemas/ErrorModel'examples:
ViewToLoadDoesNotExist:
$ref: '#/components/examples/NoSuchViewError'

[spr0els]

Good catch. I agree, this should be a NoSuchViewException.

There are a two tests for tables in the same file that also return the wrong error type. These should return NoSuchTableException.

iceberg-python/tests/catalog/test_rest.py

Lines 1157 to 1160 in b67b724

	"error": {
	"message": "Table does not exist: examples.does_not_exists in warehouse 8bcb0838-50fc-472d-9ddb-8feb89ef5f1e",
	"type": "NoSuchNamespaceErrorException",
	"code": 404,

https://github.com/apache/iceberg/blob/8d0508308cd068778897c06d3a802740ed4305a6/open-api/rest-catalog-open-api.yaml#L1009-L1018

iceberg-python/tests/catalog/test_rest.py

Lines 1218 to 1221 in b67b724

	"error": {
	"message": "Table does not exist: fokko.does_not_exists in warehouse 8bcb0838-50fc-472d-9ddb-8feb89ef5f1e",
	"type": "NoSuchNamespaceErrorException",
	"code": 404,

https://github.com/apache/iceberg/blob/8d0508308cd068778897c06d3a802740ed4305a6/open-api/rest-catalog-open-api.yaml#L1170-L1179

Should I fix all of these in one go or create a issue for the table tests?

[gabeiglio]

I think we can do them in a different PR to keep this focused on load_view

[gabeiglio]

thanks! lgtm cc: @geruh for when you can double check 😉

[geruh]

lgtm, thanks for the reviews everyone!!