add isolation level to KafkaProperties

[sparty02]

Add the "isolation.level" property as a well defined config.

[wilkinsona]

@garyrussell @artembilan Does making the isolation level configurable seem reasonable to you?

[artembilan]

I left my concern as a review, @wilkinsona .

Let's hear what @garyrussell thinks on the matter!

[artembilan]

It may make sense if it is mapped as an org.apache.kafka.common.requests.IsolationLevel enum type.
Otherwise this one indeed can be mapped as all other generic properties in the spring.kafka.consumer.properties container.

More over I think such a property description is too long. Essentially there is just enough to mention that it is mapped onto ConsumerConfig.ISOLATION_LEVEL_CONFIG).

[sparty02]

Coincedentally, I had the same two thoughts! On both accounts, I went the direction I did because of the precedent set by existing config options. I found valid values being defined in metadata instead of enums in this example and I added the description in the comment again based on all of the existing config.

Interestingly, the isolation.level property does have its doc constant exposed as public even though most other config options are private

[garyrussell]

AFAIK, there is no enum for ConsumerConfig.AUTO_OFFSET_RESET_CONFIG (certainly not used there) which is why it's handled that way.

The isolation level enum is used in ConsumerConfig so it's best to use that.

[garyrussell]

Actually, they have to lower case the enum anyway, so maybe this is the best solution after all.

IsolationLevel.READ_UNCOMMITTED.toString().toLowerCase(Locale.ROOT);

[artembilan]

Still would be better to be based on enum. That way, when they introduce a new level, we are going to be covered automatically together with content assistant in the IDE.

[garyrussell]

As discussed here.

Spring Boot auto-configuration supports all HIGH importance properties, some selected MEDIUM and LOW properties, and any properties that do not have a default value.

Only a subset of the properties supported by Kafka are available directly through the KafkaProperties class. If you wish to configure the producer or consumer with additional properties that are not directly supported, use the following properties:

spring.kafka.properties.prop.one=first
spring.kafka.admin.properties.prop.two=second
spring.kafka.consumer.properties.prop.three=third
spring.kafka.producer.properties.prop.four=fourth
spring.kafka.streams.properties.prop.five=fifth

So spring.kafka.consumer.properties.isolation.level=read_committed is all you need.

OTOH, it is nice to have content assist in IDEs so I have no objection (subject to Artem's suggestion of using the enum so that you get content-assist on valid values).

However, as we discussed when we added the initial support in boot, there are so many kafka properties, we can't make them all first class citizens.

[wilkinsona]

Thank you, @garyrussell and @artembilan. Having had my memory refreshed about the myriad properties with which Kafka can be configured, I'm tempted to leave the line drawn where it is and decline this one. Let's see what the rest of the team thinks before making a decision.

[sparty02]

To clarify, I'm aware of the ability to set any arbitrary properties through spring.kafka.consumer.* and friends. But as mentioned in the comment above, it's useful to have autocomplete in IDEs to nudge you in the right direction. In an ideal world, the entire set of consumer configs (and others) would be available, but it seems that would only work if there was a process to generate that code/etc. That being said, the current subset seems arbitrary and I'm not sure what the rationale would be for freezing it now vs letting it incrementally grow over time (until, if ever, a process was figured out to codegen based off the official config api).

[wilkinsona]

We've discussed this today and come to the conclusion that we don't like the current halfway-house situation and would like to support all of Kafka's properties. That's a large piece of work that'll happen post-2.2 (if at all). In the meantime, given that planned direction, we're happy to add a property for the isolation level.

[wilkinsona]

Thanks very much for making your first contribution to Spring Boot, @sparty02. The changes have been merged into master with a polish commit that makes use of the IsolationLevel enum, removes the now redundant additional metadata, and tests the new property.