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/12/19 19:40:39 UTC

[GitHub] [kafka] mjsax opened a new pull request, #13022: MINOR: fix expected version in streams upgrade test

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

   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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] suhas-satish commented on a diff in pull request #13022: MINOR: fix expected version in streams upgrade test

Posted by GitBox <gi...@apache.org>.
suhas-satish commented on code in PR #13022:
URL: https://github.com/apache/kafka/pull/13022#discussion_r1052581567


##########
tests/kafkatest/tests/streams/streams_upgrade_test.py:
##########
@@ -506,7 +508,7 @@ def do_rolling_bounce(self, processor, counter, current_generation):
                     self.upgraded_processors.append(processor)
 
                     # checking for the dev version which should be the only SNAPSHOT
-                    log_monitor.wait_until("Kafka version.*" + self.base_version_number + ".*SNAPSHOT",
+                    log_monitor.wait_until(kafka_version_str,

Review Comment:
   does kafka_version_str already have the SNAPSHOT suffix or did something change in the naming convention on  dev? Approving pre-emptively. 



-- 
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] mjsax commented on pull request #13022: MINOR: fix expected version in streams upgrade test

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

   Merged to `trunk` and cherry-picked to `3.4` branch.


-- 
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] mjsax commented on a diff in pull request #13022: MINOR: fix expected version in streams upgrade test

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


##########
tests/kafkatest/tests/streams/streams_upgrade_test.py:
##########
@@ -506,7 +508,7 @@ def do_rolling_bounce(self, processor, counter, current_generation):
                     self.upgraded_processors.append(processor)
 
                     # checking for the dev version which should be the only SNAPSHOT
-                    log_monitor.wait_until("Kafka version.*" + self.base_version_number + ".*SNAPSHOT",
+                    log_monitor.wait_until(kafka_version_str,

Review Comment:
   I am not 100% sure, but I assume something upstream changed and a new version is passed in.
   
   Guess it depends what artifact are used to run the test -- if we build from release branches, the `SNAPSHOT` suffix should be contained, but if we run the test with release artifacts, the `SNAPSHOT` might have been removed already.



-- 
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] mjsax merged pull request #13022: MINOR: fix expected version in streams upgrade test

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


-- 
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] mjsax commented on pull request #13022: MINOR: fix expected version in streams upgrade test

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

   System test run: https://jenkins.confluent.io/job/system-test-kafka-branch-builder/5317/


-- 
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] lucasbru commented on a diff in pull request #13022: MINOR: fix expected version in streams upgrade test

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


##########
tests/kafkatest/tests/streams/streams_upgrade_test.py:
##########
@@ -489,6 +489,8 @@ def do_rolling_bounce(self, processor, counter, current_generation):
         first_other_node = first_other_processor.node
         second_other_node = second_other_processor.node
 
+        kafka_version_str = self.get_version_string(self.base_version_number)

Review Comment:
   This will work, but make the check slightly weaker, since it will never check for `SNAPSHOT`, even if I am currently on a snapshot version.
   ```suggestion
           kafka_version_str = self.get_version_string(str(DEV_VERSION))
   ```



-- 
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] mjsax commented on pull request #13022: MINOR: fix expected version in streams upgrade test

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

   Yes, just retriggered as https://jenkins.confluent.io/job/system-test-kafka-branch-builder/5321/ (5320 passed) after latest change.


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