API SchemaRegistryClient updates #24529
Conversation
ghost
added
the
| * | ||
| * @return The content of the schema. | ||
| */ | ||
| public String getContent() { |
There was a problem hiding this comment.
Since we are calling this schemaDefinition in the registerSchema API, we should use the same terminology here.
There was a problem hiding this comment.
Good call. I relayed this to @deyaaeldeen .
| * | ||
| * @param schemaId the schema id | ||
| * @param schemaFormat type of schema, e.g. avro, json | ||
| * @param schemaName name of the schema. |
There was a problem hiding this comment.
Why did we remove the schemaName?
There was a problem hiding this comment.
AFAIK, none of the other languages had this parameter. It used to always return null, and I had to add some regex magic to parse it from the returned headers.
ex: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/schemaregistry/Azure.Data.SchemaRegistry/src/SchemaProperties.cs
There was a problem hiding this comment.
This might be worth asking the service team to include schemaName in their response.
There was a problem hiding this comment.
@deyaaeldeen . IIRC, we asked the service team about this?
There was a problem hiding this comment.
I do not believe but we can discuss this in our next sync. For now, all languages do not have it so let's remove it in Java as well.
|
API changes have been detected in |
| * | ||
| * @param schemaId the schema id | ||
| * @param schemaFormat type of schema, e.g. avro, json | ||
| * @param schemaName name of the schema. |
There was a problem hiding this comment.
This might be worth asking the service team to include schemaName in their response.
| */ | ||
| public SchemaRegistrySchema(SchemaProperties properties, String schemaDefinition) { | ||
| this.properties = properties; | ||
| this.content = schemaDefinition; |
There was a problem hiding this comment.
nit: use the same name for instance property too
| this.content = schemaDefinition; | |
| this.schemaDefinition = schemaDefinition; |
|
API changes have been detected in API changes - public SchemaRegistrySchema(SchemaProperties properties, String content)
- public String getContent()
+ public SchemaRegistrySchema(SchemaProperties properties, String schemaDefinition)
+ public String getSchemaDefinition() |
Updates based on API Review:
getSchemaIdtogetSchemaPropertieswithResponsemethods back to schema clients.contents->schemaDefinitionRelated: #24221
API View: https://apiview.dev/Assemblies/Review/0615cbf9af5e4f5caa0b113ae279492e