WIP - Do not Merge: Added support for AWS Glue Schema registry

[nicklester]

Added support to enable the AWS Glue Schema Registry

[davideicardi]

Thank you @nicklester !
It LGTM, maybe I would only prefer to have the AWS Glue implementation a little more "isolated" ... What do you think @Bert-R ?

[Bert-R]

I've added a few small comments.
I agree with @davideicardi that a bit more isolation and genericity would be helpful.

What about this:

• Add a property schemaregistry.type, with possible values generic and aws-glue. Default is generic.
• Extend SchemaRegistryProperties with a check: if type != 'generic', then the properties connect and auth should not be set.
• Enhance GlueSchemaRegistryProperties in a similar way and remove isConfigured. Als add a validation that the mandatory properties are set if this registry type is configured.
• Extend AvroMessageDeserializer with a map of deserializer factories, keyed by the registry type.

With that, we're prepared for a future PR that adds support for Microsoft's SchemaRegistryApacheAvroSerializer

[Bert-R]

Move this one down, to maintain the same order as the parameter list

[Bert-R]

Suggested change

	--schemaregistry.glue.region=us-east-1 --schemaregistry.glue.registryName=demo-registry
	--schemaregistry.glue.region=us-east-1
	--schemaregistry.glue.registryName=demo-registry

Besides this, also document the other (apparently optional) properties.

[Bert-R]

Suggested change

	if(StringUtils.isNotEmpty(awsEndpoint))
	config.put(AWSSchemaRegistryConstants.AWS_ENDPOINT, awsEndpoint);
	if (StringUtils.isNotEmpty(awsEndpoint)) {
	config.put(AWSSchemaRegistryConstants.AWS_ENDPOINT, awsEndpoint);
	}

We'll need to establish and publish a code style, but from what I see, curly braces are used around all blocks.

[nicklester]

good suggestiosn - sorry slow response. Busy work wise. Will take a look shortly

[nicklester]

I've added a few small comments. I agree with @davideicardi that a bit more isolation and genericity would be helpful.

What about this:

• Add a property schemaregistry.type, with possible values generic and aws-glue. Default is generic.
• Extend SchemaRegistryProperties with a check: if type != 'generic', then the properties connect and auth should not be set.
• Enhance GlueSchemaRegistryProperties in a similar way and remove isConfigured. Als add a validation that the mandatory properties are set if this registry type is configured.
• Extend AvroMessageDeserializer with a map of deserializer factories, keyed by the registry type.

With that, we're prepared for a future PR that adds support for Microsoft's SchemaRegistryApacheAvroSerializer

Was just a quick question - (as Java is not my 1st or even 2nd language!). Isn't using a factory going to mean we need to pass the same types as arguments, but the generic and glue configs are different, and in different hierarchies). Apologies if I've missed something obvious :)

[Bert-R]

No problem! As always, there are multiple ways to approach it. I'd do a slightly bigger refactor as you find in
the attached patch. Note that I have not done any testing. The constants for the schema registry type would need to be moved from MessageDeserializerFactory to a SchemaRegistryConfiguration, to use them to test the possible values for type.