Skip to content

KAFKA-9290: Update IQ related JavaDocs#8114

Merged
mjsax merged 25 commits intoapache:trunkfrom
highluck:timestamp-doc
May 8, 2020
Merged

KAFKA-9290: Update IQ related JavaDocs#8114
mjsax merged 25 commits intoapache:trunkfrom
highluck:timestamp-doc

Conversation

@highluck
Copy link
Copy Markdown
Contributor

Update IQ related JavaDocs

Committer Checklist (excluded from commit message)

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

@highluck highluck requested a review from mjsax February 13, 2020 22:47
@mjsax
Copy link
Copy Markdown
Member

mjsax commented Feb 14, 2020

Thanks for the PR @highluck -- this overlaps with #7848 but the other PR seems to be abandoned? Hence, I would just more forward with your PR.

There are some other changes in #7848 (changing KeyValueStore to ReadOnlyKeyValueStore and adding missing generic types) -- also the class JavaDoc of KTable has a change in it. Can you include those changes in your PR?

\cc @miroswan who is the author of #7848 -- let us know if you are still interested in finishing your PR (we don't want to "take it away" from you).

@mjsax mjsax added the streams label Feb 14, 2020
@miroswan
Copy link
Copy Markdown

Thanks for the PR @highluck -- this overlaps with #7848 but the other PR seems to be abandoned? Hence, I would just more forward with your PR.

There are some other changes in #7848 (changing KeyValueStore to ReadOnlyKeyValueStore and adding missing generic types) -- also the class JavaDoc of KTable has a change in it. Can you include those changes in your PR?

\cc @miroswan who is the author of #7848 -- let us know if you are still interested in finishing your PR (we don't want to "take it away" from you).

@mjsax I apologize. Please feel free to reassign.

@highluck
Copy link
Copy Markdown
Contributor Author

highluck commented Feb 14, 2020

@mjsax
thanks for review

i'm update docs!

@miroswan
thanks!

@highluck
Copy link
Copy Markdown
Contributor Author

@mjsax
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.

Sorry for the review delay. LGTM! Will merge after Jenkins passed.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 19, 2020

Retest this please

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 20, 2020

Build failed with checkstyle error:

[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java:34:8: Unused import - org.apache.kafka.streams.processor.ProcessorContext. [UnusedImports]
11:59:28 [ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/src/test/java/org/apache/kafka/streams/state/internals/RocksDBStoreTest.java:163:21: '=' is not preceded with whitespace. [WhitespaceAround]

@highluck Can you update the PR?

@highluck
Copy link
Copy Markdown
Contributor Author

Thank you for reivew
Yes ok!

@highluck
Copy link
Copy Markdown
Contributor Author

@mjsax

It was a hotfix!
#8515

# Conflicts:
#	streams/src/main/java/org/apache/kafka/streams/StreamsBuilder.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/CogroupedKStream.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/KGroupedStream.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/KGroupedTable.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/KTable.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedCogroupedKStream.java
#	streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java
* ReadOnlyKeyValueStore<String, Long> localStore = streams.store(queryableStoreName, QueryableStoreTypes.<String, Long>keyValueStore());
* String key = "some-key";
* Long valueForKey = localStore.get(key); // key must be local (application state is shared over all running Kafka Streams instances)
* ReadOnlyKeyValueStore<K,ValueAndTimestamp<V>> localStore = streams.store(queryableStoreName, QueryableStoreTypes.<K, ValueAndTimestamp<V>>timestampedKeyValueStore());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: insert space between generics (here and elsewhere)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok! thanks :)

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Apr 20, 2020

Ah. Thanks for pointing out the hotfix @highluck. Can you address Sophie's comment?

Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsBuilder.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsBuilder.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsBuilder.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/StreamsBuilder.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/KGroupedStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/KGroupedStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java Outdated
Comment thread streams/src/main/java/org/apache/kafka/streams/kstream/TimeWindowedKStream.java Outdated
@mjsax mjsax merged commit 133c2ed into apache:trunk May 8, 2020
@mjsax
Copy link
Copy Markdown
Member

mjsax commented May 8, 2020

I removed the blanks you inserted on the wrong place in a previous commit but not not remove in your latest commit.

Thanks for the PR! Merged to trunk.

@highluck
Copy link
Copy Markdown
Contributor Author

highluck commented May 8, 2020

@mjsax
thanks!!

Kvicii pushed a commit to Kvicii/kafka that referenced this pull request May 8, 2020
* 'trunk' of github.com:apache/kafka:
  KAFKA-9290: Update IQ related JavaDocs (apache#8114)
  KAFKA-9928: Fix flaky GlobalKTableEOSIntegrationTest (apache#8600)
  KAFKA-6145: Set HighAvailabilityTaskAssignor as default in streams_upgrade_test.py (apache#8613)
  KAFKA-9667: Connect JSON serde strip trailing zeros (apache#8230)
  MINOR: Log4j Improvements on Fetcher (apache#8629)
jwijgerd pushed a commit to buxapp/kafka that referenced this pull request May 14, 2020
Reviewers: A. Sophie Blee-Goldman <sophie@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.

5 participants