suvodeep-pyne commented on pull request #5769: URL: https://github.com/apache/incubator-pinot/pull/5769#issuecomment-667299037
Hey @jasonyanwenl Thanks for getting back to me. Just wanted to understand some things better: - I assume that the online worker must be on when such an API request is issued? What is the behavior if that is not the case? - I wanted to understand the necessity of having 2 TaskDrivers. How is an online TaskDriver different in behavior than an offline task driver? Same question for online detection task vs offline detection task. - Is this about prioritizing online tasks? If yes, have we considered having a priority attached to a task? Some comments on readability that have helped me: - First of all thank you! I would really encourage having small PRs. It just makes it really easy to review and push commits faster if the changes are small and focussed. Imagine not having the context and reviewing a 1300 line PR. - This can be across multiple files but addressing a single/few related issue(s). - Also smaller methods help. same logic: easier to read, refactor, test. And less code duplication. - Smaller classes. Starting with an 800 line Resource class makes it bit hard to read, maintain. Apart this, I really like the utility of these APIs. Thanks for adding this! Regards ---------------------------------------------------------------- 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 --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
