Core: IRC: Implement Events Endpoint Request & Response Objects
implementing Request and Response objects for /events and other relevant objects, including serialization & de-serialization tests
Spec draft PR: https://github.com/apache/iceberg/pull/12584
Closes #13580
@c-thiel can you please take a look? I will add the tests by tmrw
PS: This is my first iceberg PR, pardon any noob mistakes :upside_down_face:
@c-thiel I have a couple of queries regarding the implementation
- I took inspiration from Namespace class for implementing
CatalogObject. So, should we support EMPTY_CATALOG_OBJECT too, just like EMPTY_NAMESPACE? - Should we add validation checks like
null,emptychecks for required fields in operations, request and response object classes? - I am assuming in QueryEventsRequest, we would only be supporting standard operation types and 'custom'. We would not support filtering on custom operation types like 'x-<some-op>'
@aheev you might want to look at previous PRs that were introducing request/response classes for REST-specific things, such as https://github.com/apache/iceberg/commit/1cb88a64f8a065c87eb875cf08cfc70941a2fd05 or https://github.com/apache/iceberg/commit/f19643a93f5dac99bbdbc9881ef19c89d7bcd3eb. All of the request/response classes need to be in their respective packages and also have a separate JSON parser class with testing. Those JSON parsers then also need to be registered for ser/de in RESTSerializers. Additionally, all of the new types need to live under the rest package in iceberg-core
@nastra addressed your comments
Still some questions linger
- What should be the structure of CatalogObject? Should it be same as
Namespaceor just astringcontaining the fully specified name of the object? - I am assuming in QueryEventsRequest, we would only be supporting filters on standard operation types and 'custom'. We would not support filtering on custom operation types like 'x-
' - Do we plan to use CatalogObject and CatalogObjectUuid other than
/eventsin future? If not, I can just move them it torest/events
CC: @nastra @c-thiel