[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1320112618 Thanks @C0urante! -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1212026332 Sorry for the wait on this—it looks like work to switch the test to Mockito is in flight now, so I’ll hang tight for now and rebase once that lands. I do have an account on the ASF JIRA; dstelljes is my username there as well. I tried to self-assign when I picked this up but didn’t have the permissions for it. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1196168380 Hmm, if this is what you see than I have the same thing: ``` No tests found for given includes: [**/*Suite.class](exclude rules) [org.apache.kafka.connect.runtime.distributed.DistributedHerderTest.testConnectorConfigUpdate](--tests filter) ``` It seems to be the `@PrepareForTest`s at the method level. I also tried removing the `@PrepareForTest` at the class level and copying it onto each test to rule out issues with class/method level combination and got the same result. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1192603682 @rhauch I see you merged a couple Connect pulls lately; can I bug you for this one as well? -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1161765328 Cool, yeah, changing those annotations got everything working on my side too. (Sorry for the scare!) I took your suggestion verbatim and everything appears to work; I definitely like that better than duplicating the try/catch everywhere. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] dstelljes commented on pull request #12295: KAFKA-13586: Prevent exception thrown during connector update from crashing distributed herder
dstelljes commented on PR #12295: URL: https://github.com/apache/kafka/pull/12295#issuecomment-1160666379 Hey @C0urante, thanks for the comments! I just added a variation on the update test for this, but it’s causing `testTaskRequestedZombieFencingForwardedToLeader` to hang locally. I’m still not sure what I messed up, but I’m pushing it up as is since the zombie fencing tests are new and you’ll probably be able to spot the problem much faster than I will. Ideally, I think `ConfigException` itself would be checked. That way, `ConfigProvider::get` implementations could throw `ConfigException` for invalid paths, missing keys, etc., and other runtime exceptions could be assumed to be a provider defect or some other worker-level issue. That ship has probably sailed though... It seems fair to wrap everything in `startConnector` and funnel every exception to the callback since `reconfigureConnector` does the same thing. In `getConnectorStartingCallable`, immediate failures are passed to `onFailure`; is that something we’d have to account for? -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
