You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/21 14:01:06 UTC

[GitHub] [kafka] ahmedsobeh opened a new pull request, #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

ahmedsobeh opened a new pull request, #12670:
URL: https://github.com/apache/kafka/pull/12670

   This PR makes the following changes:
   
   - Moves the only test in StateRestorationIntegrationTest into RestoreIntegrationTest
   - Deletes Merge StateRestorationIntegrationTest into RestoreIntegrationTest
   


-- 
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

[GitHub] [kafka] guozhangwang commented on pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on PR #12670:
URL: https://github.com/apache/kafka/pull/12670#issuecomment-1255601105

   Thanks @ahmedsobeh ! Will take a look.


-- 
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

[GitHub] [kafka] ahmedsobeh commented on pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
ahmedsobeh commented on PR #12670:
URL: https://github.com/apache/kafka/pull/12670#issuecomment-1255666842

   > Thanks @ahmedsobeh ! Will take a look.
   
   had a small import issue that I just fixed, should be good now.


-- 
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

[GitHub] [kafka] guozhangwang commented on pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on PR #12670:
URL: https://github.com/apache/kafka/pull/12670#issuecomment-1255735216

   There's still jenkins failures: 
   
   ```
   [2022-09-23T00:16:58.037Z] [ant:checkstyle] [ERROR] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-12670/streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:161:22: Name 'APPLICATION_ID' must match pattern '^[a-z][a-zA-Z0-9]*$'. [LocalFinalVariableName]
   
   [2022-09-23T00:16:58.037Z] [ant:checkstyle] [ERROR] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-12670/streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:162:22: Name 'STATE_STORE_NAME' must match pattern '^[a-z][a-zA-Z0-9]*$'. [LocalFinalVariableName]
   
   [2022-09-23T00:16:58.037Z] [ant:checkstyle] [ERROR] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-12670/streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:163:22: Name 'INPUT_TOPIC' must match pattern '^[a-z][a-zA-Z0-9]*$'. [LocalFinalVariableName]
   
   [2022-09-23T00:16:58.037Z] [ant:checkstyle] [ERROR] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-12670/streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:164:22: Name 'OUTPUT_TOPIC' must match pattern '^[a-z][a-zA-Z0-9]*$'. [LocalFinalVariableName]
   
   [2022-09-23T00:16:58.037Z] [ant:checkstyle] [ERROR] /home/jenkins/jenkins-agent/workspace/Kafka_kafka-pr_PR-12670/streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:168:20: Variable 'streamsConfiguration' should be declared final. [FinalLocalVariable]
   ```


-- 
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

[GitHub] [kafka] ahmedsobeh commented on a diff in pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
ahmedsobeh commented on code in PR #12670:
URL: https://github.com/apache/kafka/pull/12670#discussion_r978335459


##########
streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:
##########
@@ -97,11 +102,13 @@ public class RestoreIntegrationTest {
 
     public static final EmbeddedKafkaCluster CLUSTER = new EmbeddedKafkaCluster(NUM_BROKERS);
 
+

Review Comment:
   Used a formatter that seems to have added a couple of few lines. Removed 👍 



-- 
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

[GitHub] [kafka] guozhangwang commented on a diff in pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on code in PR #12670:
URL: https://github.com/apache/kafka/pull/12670#discussion_r978222365


##########
streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:
##########
@@ -97,11 +102,13 @@ public class RestoreIntegrationTest {
 
     public static final EmbeddedKafkaCluster CLUSTER = new EmbeddedKafkaCluster(NUM_BROKERS);
 
+

Review Comment:
   Are these new lines intentional? If not could you revert them?



##########
streams/src/test/java/org/apache/kafka/streams/integration/RestoreIntegrationTest.java:
##########
@@ -156,14 +228,16 @@ public void shouldRestoreStateFromSourceTopic(final boolean stateUpdaterEnabled)
         final Properties props = props(stateUpdaterEnabled);
         props.put(StreamsConfig.TOPOLOGY_OPTIMIZATION_CONFIG, StreamsConfig.OPTIMIZE);
 
-        // restoring from 1000 to 4000 (committed), and then process from 4000 to 5000 on each of the two partitions
+        // restoring from 1000 to 4000 (committed), and then process from 4000 to 5000

Review Comment:
   The newlines introduced below till line 496 seems to be auto from IDE, could you revert them if they are not intentional?



-- 
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

[GitHub] [kafka] guozhangwang merged pull request #12670: KAFKA-14239: Merge StateRestorationIntegrationTest into RestoreIntegrationTest

Posted by GitBox <gi...@apache.org>.
guozhangwang merged PR #12670:
URL: https://github.com/apache/kafka/pull/12670


-- 
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