Skip to content

KAFKA-7855: Kafka Streams Maven Archetype quickstart fails to compile out of the box#6194

Merged
mjsax merged 4 commits intoapache:trunkfrom
aurlien:KAFKA-7855_make_maven_archetype_compile
Mar 16, 2019
Merged

KAFKA-7855: Kafka Streams Maven Archetype quickstart fails to compile out of the box#6194
mjsax merged 4 commits intoapache:trunkfrom
aurlien:KAFKA-7855_make_maven_archetype_compile

Conversation

@aurlien
Copy link
Copy Markdown
Contributor

@aurlien aurlien commented Jan 24, 2019

In the LineSplit.java example, the untyped KStream is resolved to KStream<Object, Object>. value.split(..) then fails to build, because value is of type Object.

Other possible solutions would be to add the type to the builder:
builder.<String, String>stream(...).flatmap(..).to(...), or builder.stream(..., Consumed.with(Serdes.String(), Serdes.String())).

This change matches the code in the tutorial.

WIP: It should be tested during compile that the Java resource files compiles. However, I haven't found any way to achieve this. Feedback appreciated!

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@guozhangwang
Copy link
Copy Markdown
Contributor

@aurlien thanks for the PR! Can you try build the project just as a normal mvn one and see if it compiles?

@aurlien
Copy link
Copy Markdown
Contributor Author

aurlien commented Jan 24, 2019

Hi @guozhangwang, thanks for reaching out.

Trying to compile the quickstart project (mvn clean compile), I get the following error: Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:1.6:sign (sign-artifacts) on project streams-quickstart: Unable to execute gpg command: Error while executing process. Cannot run program "gpg": error=2, No such file or directory

I haven't managed to fix this yet, but guess that has something to do with my local setup?

However, making the same change as this PR does, to the output of the following, makes the built example project compile:

mvn archetype:generate \
    -DarchetypeGroupId=org.apache.kafka \
    -DarchetypeArtifactId=streams-quickstart-java \
    -DarchetypeVersion=2.1.0 \
    -DgroupId=streams.examples \
    -DartifactId=streams.examples \
    -Dversion=0.1 \
    -Dpackage=myapps

Please tell me if I'm doing something wrong here - this is my first Kafka PR, and I'm eager to learn :)

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Jan 26, 2019

@aurlien Thanks for the PR. And welcome to the community!

I personally would prefer the solution to specify the type as builder.<String, String>stream(...) -- the reason it, that many developers don't know that this is possible. Thus, using builder.<String, String>stream(...) has an additional educational value IMHO :) -- I don't insist on it and the current PR LGTM, too.

About the error: yes, it seems you don't have gpg installed or not setup correctly -- maven can't find it.

@guozhangwang I am wondering why this was not detected via tests. Seems we don't have a test pipeline for this. I am not sure atm how hard it would be to get test coverage for this? Thoughts?

@mjsax mjsax added the streams label Jan 27, 2019
@guozhangwang
Copy link
Copy Markdown
Contributor

@guozhangwang I am wondering why this was not detected via tests. Seems we don't have a test pipeline for this. I am not sure atm how hard it would be to get test coverage for this? Thoughts?

It is a bit hard.. note for architype project, it declares the streams jar as external dependency, so to test this we need to have a unit test that takes in the streams jar. Maybe we can have a system test style case, like StreamsUpgradeTest but it should not be part of o.a.k. package.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 7, 2019

@guozhangwang What is the conclusion? We skip adding a test and merge?

@guozhangwang
Copy link
Copy Markdown
Contributor

Yeah although it is not ideal I felt it is very hard, if possible to add a test covering similar issues as part of AK, since it would involve adding a new procedure as part of unit test that 1) download an artifact, 2) mvn compile on that artifact. We can probably add a script doing so and add it as part of a jenkins job, but would hard to be part of ./gradlew test cmd.

@aurlien
Copy link
Copy Markdown
Contributor Author

aurlien commented Mar 11, 2019

Thanks for the feedback! And sorry for the late reply, for some reason this conversation ended in my spam folder.

If you prefer, I can add the type to the builder - I see your point about the educational value.

@guozhangwang
Copy link
Copy Markdown
Contributor

cc @mjsax for final reviews.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 11, 2019

I just tested this locally. It's does still not compile, because we also need to import KStream.

I leave it up to you how you fix it -- either add the import or get rid of KStream and add the type to the builder.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 14, 2019

@aurlien Any update on this?

@aurlien
Copy link
Copy Markdown
Contributor Author

aurlien commented Mar 14, 2019

@mjsax Sorry for the delay, I will fix it tomorrow!

@aurlien
Copy link
Copy Markdown
Contributor Author

aurlien commented Mar 15, 2019

@mjsax I updated to use your recommended builder.<String, String> syntax.

However, I couldn't get the previous solution to fail, when compiling this project. I ran > mvn clean compile install -Dgpg.skip in the kafka/streams/quickstart/java directory. Am i doing something wrong?

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 15, 2019

The command you execute, creates the archetype. It only fails if you use the archetype, ie, you need to do a second step:

mvn archetype:generate \
    -DarchetypeCatalog=local \
    -DarchetypeGroupId=org.apache.kafka \
    -DarchetypeArtifactId=streams-quickstart-java \
    -DarchetypeVersion=2.3.0-SNAPSHOT \
    -DgroupId=streams.examples \
    -DartifactId=streams.examples \
    -Dversion=0.1 \
    -Dpackage=myapps

cd streams.examples

mvn compile 

The creates a new project based on the archetype and you cannot compile this project.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 15, 2019

Java8 Passed. Java11 failed with flaky tests.

Retest this please.

Copy link
Copy Markdown
Member

@mjsax mjsax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Will merge after Jenkins passed.

@mjsax mjsax changed the title KAFKA-7855: [WIP] Kafka Streams Maven Archetype quickstart fails to compile out of the box KAFKA-7855: Kafka Streams Maven Archetype quickstart fails to compile out of the box Mar 15, 2019
@mjsax mjsax merged commit fd5c084 into apache:trunk Mar 16, 2019
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 16, 2019

Thanks for the fix @aurlien!

mjsax pushed a commit that referenced this pull request Mar 16, 2019
… out of the box (#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
mjsax pushed a commit that referenced this pull request Mar 16, 2019
… out of the box (#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
mjsax pushed a commit that referenced this pull request Mar 16, 2019
… out of the box (#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Mar 16, 2019

Merged to trunk and cherry-picked to 2.2, 2.1, and 2.0.

mjsax pushed a commit that referenced this pull request Mar 16, 2019
… out of the box (#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
mjsax pushed a commit to mjsax/kafka that referenced this pull request Mar 16, 2019
… out of the box (apache#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
mjsax pushed a commit that referenced this pull request Mar 16, 2019
… out of the box (#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* apache/trunk:
  MINOR: Retain public constructors of classes from public API (apache#6455)
  KAFKA-8118; Ensure ZK clients are closed in tests, fix verification (apache#6456)
  KAFKA-7813: JmxTool throws NPE when --object-name is omitted
  KAFKA-8114: Wait for SCRAM credential propagation in DelegationTokenEndToEndAuthorizationTest (apache#6452)
  KAFKA-8111; Set min and max versions for Metadata requests (apache#6451)
  KAFKA-7855: Kafka Streams Maven Archetype quickstart fails to compile out of the box (apache#6194)
  MINOR: Update code to not use deprecated methods (apache#6434)
  MINOR: Update Trogdor ConnectionStressWorker status at the end of execution (apache#6445)
  KAFKA-8091; Use commitSync to check connection failure in listener update test (apache#6450)
  KAFKA-7027: Add an overload build method in scala (apache#6373)
  MINOR: Fix typos in LogValidator (apache#6449)
  KAFKA-7502: Cleanup KTable materialization logic in a single place (apache#6174)
  KAFKA-7730; Limit number of active connections per listener in brokers (KIP-402)
  KAFKA-8091; Remove unsafe produce from dynamic listener update test (apache#6443)
  MINOR: Fix JavaDocs warnings (apache#6435)
  MINOR: Better messaging for invalid fetch response (apache#6427)
  MINOR: Use Java 8 lambdas in KStreamImplTest (apache#6430)
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
… out of the box (apache#6194)

Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants