[#2466] feat(catalog-kafka): Add code skeleton for Kafka catalog #2509
Conversation
| throw new UnsupportedOperationException( | ||
| "Hive catalog does not support topic properties metadata"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why not provide a default implementation instead of implementing a throw UnsupportedOperationException in each class?
There was a problem hiding this comment.
Because we need to draw the attention of the developers of the catalog to this interface.
There was a problem hiding this comment.
I think this will cause trouble for extensions. For example, if another xxxPropertiesMetadata is added to the interface, all subclasses will have to add the same code, isn't that troublesome?
There was a problem hiding this comment.
Maybe you could create a separate issue for this? Since tablePropertiesMetadata and filesetPropertiesMetadata have the same implementations, we should incorporate overall changes in another PR.
What would you suggest? @jerryshao
There was a problem hiding this comment.
Yes, I think we can create a separate issue to rethink the current interface of xxxPropertiesMetadata.
| "The Kafka broker(s) to connect to, allowing for multiple brokers by comma-separating them", | ||
| true /* immutable */, | ||
| false /* hidden */)); | ||
|
|
There was a problem hiding this comment.
Does this only include one attribute, with the other attributes to be added in a later PR?
There was a problem hiding this comment.
yes, other properties' metadata will be added in a later PR
|
|
||
| val copyCatalogLibs by registering(Copy::class) { | ||
| dependsOn(copyDepends, "build") | ||
| from("build/libs_all", "build/libs") |
| @Override | ||
| public Schema createSchema(NameIdentifier ident, String comment, Map<String, String> properties) | ||
| throws NoSuchCatalogException, SchemaAlreadyExistsException { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| @Override | ||
| public Schema loadSchema(NameIdentifier ident) throws NoSuchSchemaException { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| @Override | ||
| public Schema alterSchema(NameIdentifier ident, SchemaChange... changes) | ||
| throws NoSuchSchemaException { | ||
| throw new UnsupportedOperationException(); | ||
| } |
There was a problem hiding this comment.
Is it unsupported or you will do in the next PR?
There was a problem hiding this comment.
Yes, schema-relative operations will be done in another pull request.
| register("copyLibAndConfig", Copy::class) { | ||
| dependsOn(copyCatalogConfig, copyCatalogLibs) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think you still miss several integration test related codes here, are you going to do this in a separate PR?
There was a problem hiding this comment.
yes, a todo added to the below
What changes were proposed in this pull request?
This PR adds a basic code skeleton for Kafka catalog. The code skeleton doesn't address the detailed code logic, just builds a basic skeleton and metadata definition for Kafka catalog
Why are the changes needed?
Fix: #2466
Does this PR introduce any user-facing change?
No
How was this patch tested?
UT will be added later when detailed code logic is added.