search

dmwm / CRABServer

16 stars 38 forks source link

Cleanup job wrapper #8704

Closed belforte closed 2 months ago

belforte commented 2 months ago

fix #7705

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2172/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2173/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2174/artifact/artifacts/PullRequestReport.html

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2175/artifact/artifacts/PullRequestReport.html

belforte commented 2 months ago

tested with a single on on crab-dev-tw01

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2176/artifact/artifacts/PullRequestReport.html

belforte commented 2 months ago

tests

belforte commented 2 months ago

@aspiringmind-code you can also look at https://cmscrab.docs.cern.ch/crab-design/crab-job-wrapper.html#new-cmsrunanalysispy for a description of what this PR is supposed to achieve

belforte commented 2 months ago

I added one cleanup. WIll test again.

cmsdmwmbot commented 2 months ago

Jenkins results:

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-CRABServer-PR-test/2177/artifact/artifacts/PullRequestReport.html

belforte commented 2 months ago

Ready for review

belforte commented 2 months ago

@aspiringmind-code let me if you think that you do not have the time or feel not competent to review this.

aspiringmind-code commented 2 months ago

Sorry for the delay. I'll do it by tomorrow.

On Fri, 27 Sep, 2024, 4:53 pm Stefano Belforte, @.***> wrote:

@aspiringmind-code https://github.com/aspiringmind-code let me if you think that you do not have the time or feel not competent to review this.

— Reply to this email directly, view it on GitHub https://github.com/dmwm/CRABServer/pull/8704#issuecomment-2379464032, or unsubscribe https://github.com/notifications/unsubscribe-auth/BJE3IUROX72ZGQPOBMRKCP3ZYVWM5AVCNFSM6AAAAABORWZ6ASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZZGQ3DIMBTGI . You are receiving this because you were mentioned.Message ID: @.***>

aspiringmind-code commented 2 months ago

@belforte I like the use of simpler sandbox argument and the merging of two functions into one. What I still would wanna improve is removing the "moving up the directory" action. IIUC, that will not be required once $CMSSW_BASE files are put inside cmsswVersion directory inside the sandbox. Do we have an open issue regarding that? If yes, this looks good to me. Thanks for my first review opportunity.

belforte commented 2 months ago

thanks @aspiringmind-code I do not like moving files around like that eiher I thought of this solution in https://github.com/dmwm/CRABServer/issues/7705#issuecomment-2361613880 but have not created an issue Indeed I thought I did... what I have most is to expand the tarball in two directories

BEst option looks to me at some point to modify sandboxing in client so that $CMSSW_BASE is tarred into a CMSSW_X_Y_Z directory and then all files will automatically go in the proper place when the sandbox is iteratively expanded

sorry I was confused. You are correct. I have not created an issue for that. Let's do

belforte commented 2 months ago

Well... be my memory be damned. I had not created an issue for the sandbox restructuring in the client, but have already merged a PR to do it ! https://github.com/dmwm/CRABClient/issues/5332

Please have a look and let me now if that addresses your comment

aspiringmind-code commented 2 months ago

Yes, this looks perfect.

belforte commented 2 months ago

thanks you

belforte commented 2 months ago

now the problem will be to remember to cleanup the duplicated untar once new client will be in production