vvcephei commented on pull request #8913: URL: https://github.com/apache/kafka/pull/8913#issuecomment-654512188
Hey @chia7712 , I'm sorry it took me so long to take a look. I think what you've done here is fine for now, so feel free to push back, but I'm wondering if the "Service" model is the best way to do this. Like you mentioned, the reset tool isn't a long-running process, which is really what Service is for. Also, minor note: this pattern only verifies that the resetter finishes running, not that it runs successfully. What do you think about instead converting you StreamsResetter so that it doesn't extend Service, but instead just runs that command using `processor.node.account.ssh(cmd, allow_fail=False)`? Let me know if this suggestion isn't clear, I can provide more of a snippet. AFAICT, this would actually be better, since it's less code to maintain, it has less unnecessary "magic stuff" that comes along with extended StreamsTestBaseService. It just logs in, runs the command, and verifies that it exits successfully before proceeding. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
