You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mjsax (via GitHub)" <gi...@apache.org> on 2024/08/21 22:22:50 UTC
[PR] MINOR: remove get prefix for internal IQ methods [kafka]
mjsax opened a new pull request, #16954:
URL: https://github.com/apache/kafka/pull/16954
Follow up PR to https://github.com/apache/kafka/pull/16722
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16954:
URL: https://github.com/apache/kafka/pull/16954#discussion_r1730329818
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -206,29 +206,29 @@ public synchronized Collection<StreamsMetadata> getAllMetadataForTopology(final
* if streams is (re-)initializing or {@code null} if the corresponding topic cannot be found,
* or null if no matching metadata could be found.
*/
- public synchronized <K> KeyQueryMetadata getKeyQueryMetadataForKey(final String storeName,
- final K key,
- final Serializer<K> keySerializer) {
+ public synchronized <K> KeyQueryMetadata keyQueryMetadataForKey(final String storeName,
+ final K key,
+ final Serializer<K> keySerializer) {
Objects.requireNonNull(keySerializer, "keySerializer can't be null");
if (topologyMetadata.hasNamedTopologies()) {
throw new IllegalArgumentException("Cannot invoke the getKeyQueryMetadataForKey(storeName, key, keySerializer)"
+ "method when using named topologies, please use the overload that"
+ "accepts a topologyName parameter to identify the correct store");
}
- return getKeyQueryMetadataForKey(storeName,
+ return keyQueryMetadataForKey(storeName,
key,
new DefaultStreamPartitioner<>(keySerializer));
}
/**
- * See {@link StreamsMetadataState#getKeyQueryMetadataForKey(String, Object, Serializer)}
+ * See {@link StreamsMetadataState#keyQueryMetadataForKey(String, Object, Serializer)}
*/
- public synchronized <K> KeyQueryMetadata getKeyQueryMetadataForKey(final String storeName,
- final K key,
- final Serializer<K> keySerializer,
- final String topologyName) {
+ public synchronized <K> KeyQueryMetadata keyQueryMetadataForKey(final String storeName,
+ final K key,
+ final Serializer<K> keySerializer,
+ final String topologyName) {
Objects.requireNonNull(keySerializer, "keySerializer can't be null");
- return getKeyQueryMetadataForKey(storeName,
+ return keyQueryMetadataForKey(storeName,
key,
new DefaultStreamPartitioner<>(keySerializer),
topologyName);
Review Comment:
please update error message (line#257) also
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -206,29 +206,29 @@ public synchronized Collection<StreamsMetadata> getAllMetadataForTopology(final
* if streams is (re-)initializing or {@code null} if the corresponding topic cannot be found,
* or null if no matching metadata could be found.
*/
- public synchronized <K> KeyQueryMetadata getKeyQueryMetadataForKey(final String storeName,
- final K key,
- final Serializer<K> keySerializer) {
+ public synchronized <K> KeyQueryMetadata keyQueryMetadataForKey(final String storeName,
+ final K key,
+ final Serializer<K> keySerializer) {
Objects.requireNonNull(keySerializer, "keySerializer can't be null");
if (topologyMetadata.hasNamedTopologies()) {
throw new IllegalArgumentException("Cannot invoke the getKeyQueryMetadataForKey(storeName, key, keySerializer)"
Review Comment:
please update the error message also
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -112,7 +112,7 @@ public Collection<StreamsMetadata> getAllMetadata() {
* @param storeName the storeName to find metadata for
* @return A collection of {@link StreamsMetadata} that have the provided storeName
*/
- public synchronized Collection<StreamsMetadata> getAllMetadataForStore(final String storeName) {
+ public synchronized Collection<StreamsMetadata> allMetadataForStore(final String storeName) {
Objects.requireNonNull(storeName, "storeName cannot be null");
if (topologyMetadata.hasNamedTopologies()) {
throw new IllegalArgumentException("Cannot invoke the getAllMetadataForStore(storeName) method when"
Review Comment:
please update message
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java:
##########
@@ -246,7 +246,7 @@ default boolean commitRequested() {
// IQ related methods
- StateStore getStore(final String name);
+ StateStore store(final String name);
Review Comment:
Should we rename `StateManager#getStore(String)` to `StateManager#store(String)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #16954:
URL: https://github.com/apache/kafka/pull/16954
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #16954:
URL: https://github.com/apache/kafka/pull/16954#discussion_r1733869564
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java:
##########
@@ -246,7 +246,7 @@ default boolean commitRequested() {
// IQ related methods
- StateStore getStore(final String name);
+ StateStore store(final String name);
Review Comment:
Here we go: https://github.com/apache/kafka/pull/17025
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #16954:
URL: https://github.com/apache/kafka/pull/16954#discussion_r1730404618
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsMetadataState.java:
##########
@@ -206,29 +206,29 @@ public synchronized Collection<StreamsMetadata> getAllMetadataForTopology(final
* if streams is (re-)initializing or {@code null} if the corresponding topic cannot be found,
* or null if no matching metadata could be found.
*/
- public synchronized <K> KeyQueryMetadata getKeyQueryMetadataForKey(final String storeName,
- final K key,
- final Serializer<K> keySerializer) {
+ public synchronized <K> KeyQueryMetadata keyQueryMetadataForKey(final String storeName,
+ final K key,
+ final Serializer<K> keySerializer) {
Objects.requireNonNull(keySerializer, "keySerializer can't be null");
if (topologyMetadata.hasNamedTopologies()) {
throw new IllegalArgumentException("Cannot invoke the getKeyQueryMetadataForKey(storeName, key, keySerializer)"
Review Comment:
Good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #16954:
URL: https://github.com/apache/kafka/pull/16954#discussion_r1730404855
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/Task.java:
##########
@@ -246,7 +246,7 @@ default boolean commitRequested() {
// IQ related methods
- StateStore getStore(final String name);
+ StateStore store(final String name);
Review Comment:
Yes, but I will do this in a follow up PR to keep the scope of individual PRs small.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: remove get prefix for internal IQ methods [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16954:
URL: https://github.com/apache/kafka/pull/16954#issuecomment-2309029491
I open https://issues.apache.org/jira/browse/KAFKA-17420 to trace `StreamThreadTest.tearDown`, and others are known flaky.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org