pkuwm commented on issue #353: Don't ignore exceptions: adding log messages to help check exceptions. URL: https://github.com/apache/helix/issues/353#issuecomment-513992696 Sure. Agree on this: a problem could have multiple solutions. Maybe this is just a personal coding style. I just personally prefer to handle all the exceptions. If not, then think of another to solve the problem. Then closing the issue and leaving the code as is. On Mon, Jul 22, 2019 at 4:22 PM Hunter Lee <notificati...@github.com> wrote: > Just to reiterate, since this part of the code is working correctly, I > would just leave this as is. If you would like to take a jab at it just as > an exercise, that is also fine, but again there are multiple correct ways > to accomplish the same end result. > > - Hunter > > On Jul 23, 2019, 01:19, at 01:19, Hunter Lee <naren...@gmail.com> wrote: > >Yes. Like I implied in my previous comment, there are certain fields > >that only show up in valid WorkflowConfigs. I believe "workflowId" is > >one. > > > >You could accomplish the same thing with an if-else, so a better way to > >do this is to check for the workflow id field in the ZNRecord. > > > >- Hunter > > > >On Jul 23, 2019, 01:15, at 01:15, pkuwm <notificati...@github.com> > >wrote: > >>Yes, it makes sense not to expose the config details. > >> > >>But just curious, regarding this workflowConfig checking, do we have a > >>better way to validate if all the configs are workflow configs or not? > >>It is just validated in fromHelixProperty by throwing an > >>IllegalArgumentException. > >> > >>-- > >>You are receiving this because you commented. > >>Reply to this email directly or view it on GitHub: > >>https://github.com/apache/helix/issues/353#issuecomment-513989903 > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/apache/helix/issues/353?email_source=notifications&email_token=ABHSRCOIXW5HM26KF5OWVQ3QAY6MZA5CNFSM4IF4HDP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2ROE5Q#issuecomment-513991286>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABHSRCOOQNNF2WCIDJEN4ZDQAY6MZANCNFSM4IF4HDPQ> > . >
---------------------------------------------------------------- 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 With regards, Apache Git Services
