NDCMS / lobster

A userspace workflow management tool for harnessing non-dedicated resources for high-throughput workloads.
MIT License
3 stars 14 forks source link

Remove all dashboard code #646

Closed klannon closed 2 months ago

klannon commented 1 year ago

The CMS Dashboard was replaced by Monit years ago. It's not easy to interface to Monit because the monitoring data is extracted from the glidein HTCondor classads. Since we're not running in CMS Global Pool pilots, that doesn't work for us. So rather than trying to replace the deprecated dashboard calls with something, I am proposing removing it entirely.

We're looking for anything that references WMCore.Services.Dashboard.DashboardAPI

https://github.com/NDCMS/lobster/search?q=dashboardapi

ywan2 commented 1 year ago

A branch called remove-dashboard is created to implement the changes. It looks like there aren't that many things to change, or I can be mistaken. Let me know if there are any issues. See commit here.

I am also wondering since CMS Dashboard was already replaced by Monit, why are other Dashboard codes not necessary to be removed but only ones with DashboardAPI?

klannon commented 1 year ago

Great to see that you've dived right in!

Did you test to see if you can run with the modifications you made? It looks like you only removed some of the code that I was expecting. For example, in the diff you referenced above, you removed line 145 from dash.py, but I would have said that if we're dropping dashboard support, then we should also remove line 146, and with both of those lines gone, we also don't need the with block at all and you could drop line 144 as well. Then I would go and look at patch_dash to see if that could be dropped too. (It seems like it should be, because it sounds like a dashboard specific piece of code.) Then I'd also say that lines 147 and 148, plus any other lines that access self.__dash should also be dropped.

Definitely anything connected with WMCore.Services.Dashboard should be removed, not just DashboardAPI. I didn't do an exhaustive search of the code, so I'm sure there are pieces that I missed. Don't hesitate to ask questions about anything you're unsure of.

If you get to the point where you think you've removed all the relevant code, and you're able to run @hannahbnelson's test job successfully, then please make a PR with the lobster-with-conda as the base branch to merge into and ask for reviews from 1-2 other people working on this code.

klannon commented 1 year ago

Ah, one other thing! Can you rebase your branch to lobster-with-conda? You're starting from master, but we want everyone to be building on the lobster-with-conda branch that @hannahbnelson set up. If you're not sure how to do that, you can read the directions here, just DON'T delete lobster-with-conda at the end as the answer says. (That answer assumes that you're using rebase to replace one branch with another, but we're eventually going to merge your branch into lobster-with-conda, so our case is slightly different.

ywan2 commented 1 year ago

Thanks for the comments! I will rebase my branch and add the additional changes to test Hannah's job. It does look like if we want to remove everything related to WMCore.Services.Dashboard, a lot of relative things will be removed.