dongjoon-hyun commented on pull request #35663: URL: https://github.com/apache/spark/pull/35663#issuecomment-1056083608
@yangwwei . It seems that you didn't pay attention for the content of my feedbacks. > Maybe this PR gives the impression that we just need to add some annotations, My first comment was the following https://github.com/apache/spark/pull/35663#pullrequestreview-896742767 . I know you need to add more, but I don't think we need to duplicate Apache Spark like the original PR. > Sorry, but I don't think we need this at this stage. > - spark.kubernetes.scheduler.name can be used for any custom scheduler name. > - We can add any annotation names easily. My second comment was the following. > If then, please bring this back with more requirement at phase 2's real use case, @yangwwei . > I don't think see any values in this PR including that static annotation, too. My 3rd comment was > It seems that you missed my point. This is just a duplication, @yangwwei . We want to avoid this kind of boiler templating as much as possible. Again, we already support --conf spark.kubernete.driver.scheduler.name=yunikorn in the master branch when we extend for volcano. Please be specific about missing part and what we really need. My review comments are consistent and the same. Please build the missing part by utilizing the existing one instead by simply claiming that you need the same copy of code again and again. We want to be extensible and support all future custom schedulers instead of duplicating for all customer schedulers. From my perspective, I'd like to recommend to add a new feature to YuniKorn to support custom ID annotation and allow Spark jobs to specify it to `spark-app-id`. YuniKorn is not a golden standard written on rock. Please improve it more flexible first. Lastly, we are reviewing the PR piece by piece. Please make a PR meaningful, complete, and reasonable. Unfortunately, I don't think this PR meets the criteria. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
