Skip to content

Comments

MINOR: Remove deprecated KafkaStreams constructors in docs#5118

Merged
guozhangwang merged 4 commits intoapache:trunkfrom
guozhangwang:KMinor-remove-StreamsConfig-constructor
Jun 4, 2018
Merged

MINOR: Remove deprecated KafkaStreams constructors in docs#5118
guozhangwang merged 4 commits intoapache:trunkfrom
guozhangwang:KMinor-remove-StreamsConfig-constructor

Conversation

@guozhangwang
Copy link
Contributor

@guozhangwang guozhangwang commented Jun 1, 2018

Committer Checklist (excluded from commit message)

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

@guozhangwang guozhangwang requested a review from mjsax June 1, 2018 19:40
@guozhangwang
Copy link
Contributor Author

guozhangwang commented Jun 1, 2018

@bbejeck @vvcephei

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018 •
edited
Loading

Tutorial also needs an update. For example:

one can also construct a <code>StreamsConfig</code> object from the <code>props</code> map and then pass that object to the constructor,

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

Also config-streams.html:

Construct a StreamsConfig instance from the Properties instance. For example:

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

datatypes.html: By setting default SerDes via a StreamsConfig instance.

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

There is two more in dsl-api.html ("Writing to Kafka" section)

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

Also multiple in "Interactive Queries" page

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

Multiple ones in memory management: Properties streamsConfiguration -> Properties props

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

Processor API section: You can override this setting by specifying StreamsConfig.WINDOW_STORE_CHANGE_LOG_ADDITIONAL_RETENTION_MS_CONFIG in the StreamsConfig

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

security.html also uses StreamsConfig multiple times

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

Maybe update testing.html from Properties config -> Properties props ?

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

And "Writing a Streams Application": The second argument is an instance of StreamsConfig, which defines the configuration for this specific topology.

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

ProcessorContext.java has it in its JavaDoc, too: {@link org.apache.kafka.streams.StreamsConfig#StreamsConfig(Map) StreamsConfig(Map)}).

@mjsax
Copy link
Member

mjsax commented Jun 1, 2018

That should be all, if I did not miss any... Please double check. :)

@guozhangwang
Copy link
Contributor Author

guozhangwang commented Jun 1, 2018

@mjsax I made a pass over all docs file and I think I should have got all of them.

mjsax
mjsax approved these changes Jun 4, 2018
View reviewed changes
Copy link
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.

Thanks @guozhangwang! With all the HTML markup, it's hard to review... Hope I didn't miss anything...

LGTM.

bbejeck
bbejeck approved these changes Jun 4, 2018
View reviewed changes
Copy link
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

Thanks, @guozhangwang just one super-nit. LGTM

docs/streams/tutorial.html Outdated
we can now construct the Streams client with the two components we have just constructed above: the configuration map and the topology object
(one can also construct a <code>StreamsConfig</code> object from the <code>props</code> map and then pass that object to the constructor,
<code>KafkaStreams</code> have overloaded constructor functions to takes either type).
we can now construct the Streams client with the two components we have just constructed above: the configuration map and the topology object.
Copy link
Member

@bbejeck bbejeck Jun 4, 2018

Choose a reason for hiding this comment

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

super nit: configuration map -> java.util.Properties instance?

@guozhangwang guozhangwang merged commit 718d6f2 into apache:trunk Jun 4, 2018
@guozhangwang guozhangwang deleted the KMinor-remove-StreamsConfig-constructor branch June 4, 2018 20:43
ijuma added a commit to edoardocomar/kafka that referenced this pull request Jun 6, 2018
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into kip-277-fine-…
1346289 
…grained-acl-create-topics

* apache-github/trunk:
  KAFKA-5588: Remove deprecated --new-consumer tools option (apache#5097)
  MINOR: Fix for the location of the trogdor.sh executable file in the documentation. (apache#5040)
  KAFKA-6997: Exclude test-sources.jar when $INCLUDE_TEST_JARS is FALSE
  MINOR: docs should point to latest version (apache#5132)
  KAFKA-6981: Move the error handling configuration properties into the ConnectorConfig and SinkConnectorConfig classes (KIP-298)
  [KAFKA-6730] Simplify State Store Recovery (apache#5013)
  MINOR: Rename package `internal` to `internals` for consistency (apache#5137)
  KAFKA-6704: InvalidStateStoreException from IQ when StreamThread closes store (apache#4801)
  MINOR: Add missing configs for resilience settings
  MINOR: Add regression tests for KTable mapValues and filter (apache#5134)
  KAFKA-6750: Add listener name to authentication context (KIP-282) (apache#4829)
  KAFKA-3665: Enable TLS hostname verification by default (KIP-294) (apache#4956)
  KAFKA-6938: Add documentation for accessing Headers on Kafka Streams Processor API (apache#5128)
  KAFKA-6813: return to double-counting for count topology names (apache#5075)
  KAFKA-5919; Adding checks on "version" field for tools using it
  MINOR: Remove deprecated KafkaStreams constructors in docs (apache#5118)
ijuma added a commit to big-andy-coates/kafka that referenced this pull request Jun 6, 2018
@ijuma
Merge remote-tracking branch 'apache-github/trunk' into kip-290-acl-p…
6f96d0d 
…refix

* apache-github/trunk:
  KAFKA-6726: Fine Grained ACL for CreateTopics (KIP-277) (apache#4795)
  KAFKA-5588: Remove deprecated --new-consumer tools option (apache#5097)
  MINOR: Fix for the location of the trogdor.sh executable file in the documentation. (apache#5040)
  KAFKA-6997: Exclude test-sources.jar when $INCLUDE_TEST_JARS is FALSE
  MINOR: docs should point to latest version (apache#5132)
  KAFKA-6981: Move the error handling configuration properties into the ConnectorConfig and SinkConnectorConfig classes (KIP-298)
  [KAFKA-6730] Simplify State Store Recovery (apache#5013)
  MINOR: Rename package `internal` to `internals` for consistency (apache#5137)
  KAFKA-6704: InvalidStateStoreException from IQ when StreamThread closes store (apache#4801)
  MINOR: Add missing configs for resilience settings
  MINOR: Add regression tests for KTable mapValues and filter (apache#5134)
  KAFKA-6750: Add listener name to authentication context (KIP-282) (apache#4829)
  KAFKA-3665: Enable TLS hostname verification by default (KIP-294) (apache#4956)
  KAFKA-6938: Add documentation for accessing Headers on Kafka Streams Processor API (apache#5128)
  KAFKA-6813: return to double-counting for count topology names (apache#5075)
  KAFKA-5919; Adding checks on "version" field for tools using it
  MINOR: Remove deprecated KafkaStreams constructors in docs (apache#5118)
ying-zheng pushed a commit to ying-zheng/kafka that referenced this pull request Jul 6, 2018
@guozhangwang @ying-zheng
MINOR: Remove deprecated KafkaStreams constructors in docs (apache#5118)
a6c7d57 
Reviewers: Bill Bejeck <bill@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

Reviewers

@bbejeck bbejeck bbejeck approved these changes

@mjsax mjsax mjsax approved these changes

Assignees

No one assigned

Labels

streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@guozhangwang @mjsax @bbejeck