Add more examples for pyiceberg view#3414
Conversation
gabeiglio
left a comment
There was a problem hiding this comment.
Thanks for the PR! left some small comments but overall looks good
|
@gabeiglio thanks for the review. All feedbacks had been addressed now. |
| ## Create a view | ||
|
|
||
| PyIceberg supports view operations. | ||
| If the REST server does not indicate support for view endpoints, you can enable it by setting `"view-endpoints-supported": "true"`: |
There was a problem hiding this comment.
I don't think we should be encouraging this pattern. If a Catalog says they don't support view endpoints (through /v1/config), we shouldn't tell people to ignore that.
There was a problem hiding this comment.
@rambleraptor, what is your suggestion? This config property was introduced for backward compatibility. Older REST servers may not send endpoints field in the ConfigResponse even if view operations are supported.
| "uri": "http://127.0.0.1:8181", | ||
| "s3.endpoint": "http://127.0.0.1:9000", | ||
| "py-io-impl": "pyiceberg.io.pyarrow.PyArrowFileIO", | ||
| "s3.access-key-id": "admin", | ||
| "s3.secret-access-key": "password", | ||
| "view-endpoints-supported": "true", |
There was a problem hiding this comment.
Documenting view-endpoints-supported is a good idea. However, placing it under Create a view seems unusual because other view operations also require it in older REST catalog implementations.
How about keeping ## Views and mentioning this snippet there? Each section can be a H3 ###.
Or, we could add the property to configuration.md.
There was a problem hiding this comment.
So the create/load/check tables are all H2. Thus, I proceeded with the same for views. I like the idea of keeping ## Views, however, I am not sure if we should make the rest H3 as they will be a bit off compare to the table ones. WDYT?
Closes #3413
Rationale for this change
Adds more examples around view support.
Are these changes tested?
Yes. Those had been tested locally with local Polaris.
Are there any user-facing changes?
No.