You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "muralibasani (via GitHub)" <gi...@apache.org> on 2026/03/20 19:28:52 UTC
[PR] KAFKA-20328 : Adding missing test files [kafka]
muralibasani opened a new pull request, #21841:
URL: https://github.com/apache/kafka/pull/21841
Adding few missing test files.
--
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] KAFKA-20328 : Adding missing test files - streams headers [kafka]
Posted by "aliehsaeedii (via GitHub)" <gi...@apache.org>.
aliehsaeedii commented on code in PR #21841:
URL: https://github.com/apache/kafka/pull/21841#discussion_r2980354235
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBMigratingWindowStoreWithHeaders.java:
##########
@@ -53,6 +53,11 @@ public class RocksDBMigratingWindowStoreWithHeaders extends RocksDBStore impleme
static final byte[] WINDOW_STORE_HEADERS_VALUES_COLUMN_FAMILY_NAME =
"windowKeyValueWithHeaders".getBytes(StandardCharsets.UTF_8);
+ public RocksDBMigratingWindowStoreWithHeaders(final String name,
+ final String metricsScope) {
+ super(name, metricsScope);
+ }
Review Comment:
Why do we need this constructor?
--
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] KAFKA-20328 : Adding missing test files - streams headers [kafka]
Posted by "aliehsaeedii (via GitHub)" <gi...@apache.org>.
aliehsaeedii commented on code in PR #21841:
URL: https://github.com/apache/kafka/pull/21841#discussion_r2980354235
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/RocksDBMigratingWindowStoreWithHeaders.java:
##########
@@ -53,6 +53,11 @@ public class RocksDBMigratingWindowStoreWithHeaders extends RocksDBStore impleme
static final byte[] WINDOW_STORE_HEADERS_VALUES_COLUMN_FAMILY_NAME =
"windowKeyValueWithHeaders".getBytes(StandardCharsets.UTF_8);
+ public RocksDBMigratingWindowStoreWithHeaders(final String name,
+ final String metricsScope) {
+ super(name, metricsScope);
+ }
Review Comment:
Why do we need this constructor?
--
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] KAFKA-20328 : Adding missing test files - streams headers [kafka]
Posted by "aliehsaeedii (via GitHub)" <gi...@apache.org>.
aliehsaeedii commented on PR #21841:
URL: https://github.com/apache/kafka/pull/21841#issuecomment-4133404094
> > Thanks @muralibasani for adding these tests — I agree they’re valuable and were needed. The original intention of the ticket was more about verifying the public APIs (as the title suggests) that are exposed to end users; I’ve added some clarification in the ticket description. Your PR still looks useful, so we can keep it as well.
>
> Thank you @aliehsaeedii I will keep this PR open then.
>
> Will look at the ticket again to verify the apis.
I just saw this PR: https://github.com/apache/kafka/pull/21830/ which is removing some of the classes you added test for. Considering that, this PR will be merged soon, I assume you need a rebase and also remvoing the corresponding tests.
--
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] KAFKA-20328 : streams - Adding missing test files - streams headers [kafka]
Posted by "muralibasani (via GitHub)" <gi...@apache.org>.
muralibasani commented on PR #21841:
URL: https://github.com/apache/kafka/pull/21841#issuecomment-4364607794
Closing this, as I opened new pr based on the suggestion from @aliehsaeedii
--
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] KAFKA-20328 : streams - Adding missing test files - streams headers [kafka]
Posted by "muralibasani (via GitHub)" <gi...@apache.org>.
muralibasani closed pull request #21841: KAFKA-20328 : streams - Adding missing test files - streams headers
URL: https://github.com/apache/kafka/pull/21841
--
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] KAFKA-20328 : Adding missing test files - streams headers [kafka]
Posted by "muralibasani (via GitHub)" <gi...@apache.org>.
muralibasani commented on PR #21841:
URL: https://github.com/apache/kafka/pull/21841#issuecomment-4129270308
> Thanks @muralibasani for adding these tests — I agree they’re valuable and were needed. The original intention of the ticket was more about verifying the public APIs (as the title suggests) that are exposed to end users; I’ve added some clarification in the ticket description. Your PR still looks useful, so we can keep it as well.
Thank you @aliehsaeedii I will keep this PR open then.
Will look at the ticket again to verify the apis.
--
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] KAFKA-20328 : Adding missing test files - streams headers [kafka]
Posted by "muralibasani (via GitHub)" <gi...@apache.org>.
muralibasani commented on PR #21841:
URL: https://github.com/apache/kafka/pull/21841#issuecomment-4149551463
> > > Thanks @muralibasani for adding these tests — I agree they’re valuable and were needed. The original intention of the ticket was more about verifying the public APIs (as the title suggests) that are exposed to end users; I’ve added some clarification in the ticket description. Your PR still looks useful, so we can keep it as well.
> >
> >
> > Thank you @aliehsaeedii I will keep this PR open then.
> > Will look at the ticket again to verify the apis.
>
> I just saw this PR: #21830 which is removing some of the classes you added test for. Considering that, this PR will be merged soon, I assume you need a rebase and also remvoing the corresponding tests.
Sure, will wait for that pr to be merged, and then I shall rebase.
--
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