narendly commented on issue #357: [WIP] Add getWorkflows(long timeout) to TaskDriver. URL: https://github.com/apache/helix/pull/357#issuecomment-514134881 As for the timeout value of 0, the semantics you proposed are a bit confusing. Perhaps enforce that the timeout value must be greater than 0. Throw a HelixException if equal to or less than 0? Also make sure the JavaDoc includes this information. -Hunter On Jul 23, 2019, 10:29, at 10:29, pkuwm <[email protected]> wrote: >Definitely could. I thought about it. getWorkflows() calls >getWorkflows(0). >But considering future.get can also accept 0 as timeouts. If we don’t >expect 0 ms as timeout, we could definitely clean up getWorkflows() >and >call getWorkflows(0), which means no timeout. How about that? > >On Tue, Jul 23, 2019 at 1:19 AM Hunter Lee <[email protected]> >wrote: > >> Also, to follow the DRY principle - could you try to reuse the logic >we >> have in the existing getWorkflows ()? getWorkflows(long timeout) >could >> simply be a wrapper for getWorkflows () with a timeout. >> >> On Jul 23, 2019, 10:01, at 10:01, pkuwm <[email protected]> >wrote: >> >Sometimes zookeeper hangs in the getWorkflows() call, and we could >> >provide an API with a timeout to handle this at the application >level. >> >Use ExcecutorService and Future to implement the timeout function. >> >Call future.get(long timeout, TimeUnit timeUnit) to wait for >result. >> > >> >This commit resolves #326: >> >1. Add a static API parseWorkflowConfig to class WorkflowConfig. >> >The API can parse HelixProperty. >> >2. Add an API getWorkflows(long timeout) to TaskDriver. >> >You can view, comment on, or merge this pull request online at: >> > >> > https://github.com/apache/helix/pull/357 >> > >> >-- Commit Summary -- >> > >> > * Add getWorkflows(long timeout) to TaskDriver. >> > >> >-- File Changes -- >> > >> > M helix-core/src/main/java/org/apache/helix/task/TaskDriver.java >(54) >> >M >helix-core/src/main/java/org/apache/helix/task/WorkflowConfig.java >> >(16) >> > >> >-- Patch Links -- >> > >> >https://github.com/apache/helix/pull/357.patch >> >https://github.com/apache/helix/pull/357.diff >> > >> >-- >> >You are receiving this because you are subscribed to this thread. >> >Reply to this email directly or view it on GitHub: >> >https://github.com/apache/helix/pull/357 >> >> — >> You are receiving this because you authored the thread. >> Reply to this email directly, view it on GitHub >> ><https://github.com/apache/helix/pull/357?email_source=notifications&email_token=ABHSRCLYUXRFQ6JWJKWJAOTQA25KHA5CNFSM4IGBGEJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2SKCEA#issuecomment-514105616>, >> or mute the thread >> ><https://github.com/notifications/unsubscribe-auth/ABHSRCIEWXMUBUMZIUTBTALQA25KHANCNFSM4IGBGEJA> >> . >> > > >-- >You are receiving this because you commented. >Reply to this email directly or view it on GitHub: >https://github.com/apache/helix/pull/357#issuecomment-514109056
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
