ksql icon indicating copy to clipboard operation
ksql copied to clipboard

feat: Add possibility to configure value.subject.name.strategy

Open Hubbitus opened this issue 4 years ago • 11 comments

Related to #4081

Description

Also add strategy TopicNameStrategyPlain for search schemes -key for keys and for values.

Default SubjectNameStrategy is TopicNameStrategy: for any messages published to topic the schema of the message key is registered under the subject name topic-key, and the message value is registered under the subject name `topic-value.

In some cases, it is not desired to have -key, -value value suffixes. For example in case you are do not use schemas for keys (or rarely), and assume subject name in schema-registry equal to topic name is for values and topic-key for keys

That may be configured in properties file like:

value.subject.name.strategy=io.confluent.ksql.serde.avro.subject.TopicNameStrategyPlain

Please refer to the documentation there explicitly stated limitation what such configuration is not supported currently in ksqldb.

So that pull request brings it.

Also, add strategy TopicNameStrategyPlain for search schemes topic-key for keys and topic for values.

Testing done

None autotests Manual testing: configure ksql-server.properties to have line:

value.subject.name.strategy=io.confluent.ksql.serde.avro.subject.TopicNameStrategyPlain

and check it works for schemes subject in schema-registry match <topic> for values, and not <topic>-value name.

Reviewer checklist

  • [ ] Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • [ ] Ensure relevant issues are linked (description should include text like "Fixes #")

Hubbitus avatar Dec 08 '20 14:12 Hubbitus

It looks like @Hubbitus hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence. Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

ghost avatar Dec 08 '20 14:12 ghost

[clabot:check]

Hubbitus avatar Dec 08 '20 14:12 Hubbitus

@confluentinc It looks like @Hubbitus just signed our Contributor License Agreement. :+1:

Always at your service,

clabot

ghost avatar Dec 08 '20 14:12 ghost

Jenkins fail looks strange and not-related to my changes. Can (should) I deal with it?

Hubbitus avatar Dec 08 '20 15:12 Hubbitus

Thanks for the PR @Hubbitus! I'll take a look at this sometime soon and give you some feedback. In the meantime, the failure looks related to checkstyle on your changes:

[2020-12-08T15:06:30.823Z] [ERROR] /home/jenkins/workspace/confluentinc-pr_ksql_PR-6733/ksqldb-common/src/main/java/io/confluent/ksql/util/KsqlConfig.java:334: Line is longer than 100 characters (found 131). [LineLength]
[2020-12-08T15:06:30.823Z] [ERROR] /home/jenkins/workspace/confluentinc-pr_ksql_PR-6733/ksqldb-common/src/main/java/io/confluent/ksql/util/KsqlConfig.java:335: Line is longer than 100 characters (found 124). [LineLength]
[2020-12-08T15:06:30.824Z] [ERROR] /home/jenkins/workspace/confluentinc-pr_ksql_PR-6733/ksqldb-common/src/main/java/io/confluent/ksql/util/KsqlConfig.java:337: Line is longer than 100 characters (found 115). [LineLength]
[2020-12-08T15:06:30.824Z] [ERROR] /home/jenkins/workspace/confluentinc-pr_ksql_PR-6733/ksqldb-common/src/main/java/io/confluent/ksql/util/KsqlConfig.java:339: Line is longer than 100 characters (found 120). [LineLength]
[2020-12-08T15:06:30.824Z] [ERROR] /home/jenkins/workspace/confluentinc-pr_ksql_PR-6733/ksqldb-common/src/main/java/io/confluent/ksql/util/KsqlConfig.java:340: Line is longer than 100 characters (found 121). [LineLength]

As a hint, I recommend going to the pipeline steps page on Jenkins to look for the failing task: image

agavra avatar Dec 09 '20 20:12 agavra

Hello, @agavra.

Thanks for the feedback. I have truncated long lines.

Meantime is it possible for me to see such intentions from Jenkins? If I follow to build details I see build failed, and console output contains error:

java.io.IOException: cannot find current thread
	at org.jenkinsci.plugins.workflow.cps.CpsStepContext.doGet(CpsStepContext.java:300)
	at org.jenkinsci.plugins.workflow.cps.CpsBodySubContext.doGet(CpsBodySubContext.java:88)
	at org.jenkinsci.plugins.workflow.support.DefaultStepContext.get(DefaultStepContext.java:67)
	at org.jenkinsci.plugins.credentialsbinding.impl.BindingStep$Callback.finished(BindingStep.java:255)
	at org.jenkinsci.plugins.credentialsbinding.impl.BindingStep$Execution2$Callback2.finished(BindingStep.java:163)
	at org.jenkinsci.plugins.workflow.steps.GeneralNonBlockingStepExecution$TailCall.lambda$onSuccess$0(GeneralNonBlockingStepExecution.java:140)
	at org.jenkinsci.plugins.workflow.steps.GeneralNonBlockingStepExecution.lambda$run$0(GeneralNonBlockingStepExecution.java:77)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Finished: FAILURE

Meantime pipeline steps are all green.

Hubbitus avatar Dec 09 '20 21:12 Hubbitus

That's really bizarre - I've never seen that before (the error you see on build 4). I think that one is a transient Jenkins issue. I took a look at the third build result: https://jenkins.confluent.io/job/Confluentinc%20Contributors/job/ksql/job/PR-6733/3/flowGraphTable/

The sixth build seems to be fine (or namely, it seems to show the errors properly). FWIW, I recommend you run mvn checkstyle:check locally - that will make sure you pass our checkstyle rules which are somewhat strict.

agavra avatar Dec 09 '20 21:12 agavra

Good point. Thanks. Now mvn checkstyle:check passes.

Did you consider add .editorconfig file into the project?

Hubbitus avatar Dec 09 '20 21:12 Hubbitus

Yeh. And now Jenkins build link (https://jenkins.confluent.io/job/confluentinc-pr/job/ksql/job/PR-6733/7/display/redirect) lead to 404 error, unfortunately.

Probably it should be https://jenkins.confluent.io/job/Confluentinc%20Contributors/job/ksql/job/PR-6733/5/flowGraphTable/

Hubbitus avatar Dec 09 '20 21:12 Hubbitus

Did you consider add .editorconfig file into the project?

We've standardized around checkstyle plugins across most of Confluent's products (see https://github.com/confluentinc/ksql/blob/master/CONTRIBUTING.md#code-style) though .editorconfig does seem like a neat lightweight solution. I'll definitely consider that if starting my own open source project!

agavra avatar Dec 09 '20 23:12 agavra

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

cla-assistant[bot] avatar Nov 15 '23 20:11 cla-assistant[bot]