feat: fetch schema from schema-registry by schema string #308

rroesch1 · 2024-10-09T14:37:30Z

enable users to lookup the schema id for a given schema string

mostafa · 2024-10-09T14:56:53Z

Thanks for your contribution! Can you please add a JS script to the examples to test all these cases and add some Go tests?

rroesch1 · 2024-10-09T16:11:56Z

schema_registry_test.go

 						"schemaType": srclient.Avro,
 					},
 				),
 			},
 		}).Export().(*Schema)
-		assert.Equal(t, "test-subject", newSchema.Subject)
-		assert.Equal(t, 0, newSchema.Version)


fyi: the first version of a schema should have the version 1 (not 0)

When we use the client.CreateSchema function in our code, it internally calls the client.GetSchema function from the srclient library. This function interacts with the Schema Registry API endpoint GET /schemas/ids/{id}. According to the API documentation, this endpoint returns the schema associated with a specific ID but does not include the version information in its response. As a result, the version defaults to 0 when you retrieve the schema using these functions.

To obtain the version information of the schema, you should use the getSchema wrapper function like below. This function calls a different endpoint—such as GET /subjects/{subject}/versions/{version} or GET /subjects/{subject}/versions/latest—which provides both the schema and its version information. By using getSchema, you ensure that you retrieve all the necessary details about the schema, including its version.

(We can add this as a comment here)

* enable users to lookup the schema id for a given schema string

sonarcloud · 2024-10-28T07:04:27Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

mostafa

@rroesch1 Thank a lot for your contribution! 🙏

Please address the comments.

mostafa · 2024-10-28T09:11:53Z

api-docs/index.d.ts

@@ -488,7 +488,8 @@ export class SchemaRegistry {
  constructor(schemaRegistryConfig: SchemaRegistryConfig);
  /**
   * @method
-   * Get a schema from Schema Registry by version and subject.
+   * Get latest schema from Schema Registry by subject.


You only need to update the api-docs/index.d.ts file and then run these commands to generate the docs in the same directory, considering that you haven't installed npm deps:

npm i npm run validate-d-ts npm run generate-docs npm run prettify

mostafa · 2024-10-28T09:12:19Z

api-docs/docs/classes/SchemaRegistry.md

@@ -137,8 +137,10 @@ Deserializes the given data and schema into its original form.

 ▸ **getSchema**(`schema`): [`Schema`](../interfaces/Schema.md)

-**`method`**
-Get a schema from Schema Registry by version and subject.
+**`method`** Get a schema from Schema Registry


Do not change this manually. See my comment on the api-docs/index.d.ts file above.

mostafa · 2024-10-28T09:25:29Z

schema_registry.go

 		}
 	}

 	runtime := k.vu.Runtime()
 	// The client always caches the schema.
 	var schemaInfo *srclient.Schema
 	var err error
-	// Default version of the schema is the latest version.
-	if schema.Version == 0 {
+	var isLatestSchema = false;


Suggested change

var isLatestSchema = false;

var updateCache = false;

mostafa · 2024-10-28T09:25:43Z

schema_registry.go

-			schema.Subject, schema.Version)
+	} else { // fetch latest schema for given subject
+		schemaInfo, err = client.GetSchemaByVersion(schema.Subject, schema.Version)
+		isLatestSchema = true;


Suggested change

isLatestSchema = true;

updateCache = true;

mostafa · 2024-10-28T09:25:54Z

schema_registry.go

@@ -306,7 +318,7 @@ func (k *Kafka) getSchema(client *srclient.SchemaRegistryClient, schema *Schema)
 			Subject:       schema.Subject,
 		}
 		// If the Cache is set, cache the schema.
-		if wrappedSchema.EnableCaching {
+		if wrappedSchema.EnableCaching && isLatestSchema {


Suggested change

if wrappedSchema.EnableCaching && isLatestSchema {

if wrappedSchema.EnableCaching && updateCache {

mostafa · 2024-10-28T09:33:11Z

schema_registry.go

-	} else {
-		schemaInfo, err = client.GetSchemaByVersion(
-			schema.Subject, schema.Version)
+	} else { // fetch latest schema for given subject


Suggested change

} else { // fetch latest schema for given subject

} else { // fetch schema by version

mostafa · 2024-10-28T10:14:31Z

schema_registry_test.go

 						"schemaType": srclient.Avro,
 					},
 				),
 			},
 		}).Export().(*Schema)
-		assert.Equal(t, "test-subject", newSchema.Subject)
-		assert.Equal(t, 0, newSchema.Version)


When we use the client.CreateSchema function in our code, it internally calls the client.GetSchema function from the srclient library. This function interacts with the Schema Registry API endpoint GET /schemas/ids/{id}. According to the API documentation, this endpoint returns the schema associated with a specific ID but does not include the version information in its response. As a result, the version defaults to 0 when you retrieve the schema using these functions.

To obtain the version information of the schema, you should use the getSchema wrapper function like below. This function calls a different endpoint—such as GET /subjects/{subject}/versions/{version} or GET /subjects/{subject}/versions/latest—which provides both the schema and its version information. By using getSchema, you ensure that you retrieve all the necessary details about the schema, including its version.

(We can add this as a comment here)

mostafa · 2024-10-28T10:14:44Z

schema_registry_test.go

-		assert.Equal(t, "test-subject", newSchema.Subject)
-		assert.Equal(t, 0, newSchema.Version)
+		assert.Equal(t, "test-subject", newSchema1.Subject)
+		assert.Equal(t, 1, newSchema1.Version)


Suggested change

assert.Equal(t, 1, newSchema1.Version)

assert.Equal(t, 0, newSchema1.Version)

mostafa · 2024-10-28T10:15:10Z

schema_registry_test.go

+			},
+		}).Export().(*Schema)
+		assert.Equal(t, "test-subject", newSchema2.Subject)
+		assert.Equal(t, 2, newSchema2.Version)


Suggested change

assert.Equal(t, 2, newSchema2.Version)

assert.Equal(t, 0, newSchema2.Version)

mostafa · 2024-10-28T10:16:32Z

schema_registry_test.go

@@ -271,7 +271,8 @@ func TestGetSubjectNameCanUseRecordNameStrategyWithNamespace(t *testing.T) {
 // TestSchemaRegistryClientClass tests the schema registry client class.
 func TestSchemaRegistryClientClass(t *testing.T) {
 	test := getTestModuleInstance(t)
-	avroSchema := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"}]}`
+	avroSchema1 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"}]}`
+	avroSchema2 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"}, {"name":"field2","type":"int"}]}`


I tested this locally and the space causes the tests to fail.

Suggested change

avroSchema2 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"}, {"name":"field2","type":"int"}]}`

avroSchema2 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"},{"name":"field2","type":"int"}]}`

mostafa · 2024-11-22T15:00:12Z

Hey @rroesch1,

Would you update this PR with the reviewed changes?

mostafa self-requested a review October 9, 2024 14:56

rroesch1 force-pushed the schema-lookup branch from 8ffee37 to 3c65d03 Compare October 9, 2024 16:09

rroesch1 commented Oct 9, 2024

View reviewed changes

feat: fetch schema from schema-registry by schema string

f9dd1b9

* enable users to lookup the schema id for a given schema string

rroesch1 force-pushed the schema-lookup branch from 3c65d03 to f9dd1b9 Compare October 28, 2024 07:04

rroesch1 changed the title ~~feat: fetch schema from schema-regsitry by schema string~~ feat: fetch schema from schema-registry by schema string Oct 28, 2024

mostafa requested changes Oct 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fetch schema from schema-registry by schema string #308

feat: fetch schema from schema-registry by schema string #308

rroesch1 commented Oct 9, 2024

mostafa commented Oct 9, 2024 •

edited

Loading

rroesch1 Oct 9, 2024

mostafa Oct 28, 2024

sonarcloud bot commented Oct 28, 2024

mostafa left a comment

mostafa Oct 28, 2024 •

edited

Loading

mostafa Oct 28, 2024 •

edited

Loading

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa Oct 28, 2024

mostafa commented Nov 22, 2024

	if wrappedSchema.EnableCaching && isLatestSchema {
	if wrappedSchema.EnableCaching && updateCache {

	} else { // fetch latest schema for given subject
	} else { // fetch schema by version

	assert.Equal(t, 1, newSchema1.Version)
	assert.Equal(t, 0, newSchema1.Version)

	assert.Equal(t, 2, newSchema2.Version)
	assert.Equal(t, 0, newSchema2.Version)

	avroSchema2 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"}, {"name":"field2","type":"int"}]}`
	avroSchema2 := `{"type":"record","name":"Schema","namespace":"com.example.person","fields":[{"name":"field","type":"string"},{"name":"field2","type":"int"}]}`

feat: fetch schema from schema-registry by schema string #308

Are you sure you want to change the base?

feat: fetch schema from schema-registry by schema string #308

Conversation

rroesch1 commented Oct 9, 2024

mostafa commented Oct 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 28, 2024

Quality Gate passed

mostafa left a comment

Choose a reason for hiding this comment

mostafa Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

mostafa Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mostafa commented Nov 22, 2024

mostafa commented Oct 9, 2024 •

edited

Loading

mostafa Oct 28, 2024 •

edited

Loading

mostafa Oct 28, 2024 •

edited

Loading