Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/3009#issuecomment-63755914
  
    Alright, I think this should be good to review.  I addressed the memory 
leaks; if you're interested to see whether this works, try deleting any one 
cleanup-related line: it will cause a test failure, showing that the regression 
tests for this work pretty well.
    
    I also added a bunch of Selenium tests as I found bugs / formatting issues, 
so overall I think that this should be much more stable / correct now, 
especially in cases with "phantom" stages.
    
    There are still a couple of minor wishlist items that I might add if I have 
more time, including additional unit test coverage for some of the new listener 
fields (to make sure that things are _added_ properly, not just removed).  I'd 
also like to maintain a count of the total number of jobs so that we can 
explain that jobs / stages are not being displayed once some have been GC'd.  
There was already a patch to do this for the stages page, so I can just copy 
the logic here.
    
    Finally, it's always nice to add more help text and tooltips.
    
    For 1.2, though, I think this is in good shape and I wouldn't block on 
those minor tasks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to