cloud-fan commented on pull request #26624: URL: https://github.com/apache/spark/pull/26624#issuecomment-623855672
LGTM except for the app id/name. I'm still not convinced that it's working, at least @Ngone51 reported he can't see app id/name by local testing. Can you clearly point out the code that sets app id/name? You mentioned it's in DAG scheduler, can you point out which line? It's even better if you can add a test. BTW I think it's OK to ask users to set app id/name themselves by `mdc.appId/Name`. I'm good with this patch if we just remove the handling of app id/name. ---------------------------------------------------------------- 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: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
