dmwm / CRABClient

runrange
14 stars 36 forks source link

Add change dbs dataset file status fix #5204 #5241

Closed belforte closed 11 months ago

belforte commented 11 months ago

I think this is OK now. Aside from

belforte commented 11 months ago

I did a quick test on CMSSW_9 on lxplus7 (python2) and it is OK. Once have test in Jenkins will run all archs and of course I need to make sure I did not break anything !

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

belforte commented 11 months ago

thanks a lot @novicecpp I have a question for you now: would it then be better to also move HTTPRequests to ClientUtilities ? Since it is not used by CRABRest only.

belforte commented 11 months ago

I merged my branch with master dmwm repo in test-setdatasetfilesstatus branch tagged as testSetDsetFilesStatus.1 and ran Jenkins CV to check for regression

All OK !

belforte commented 11 months ago

@novicecpp I captured your comments before in this list of actions. I will add more as needed once you answer my questions. Feel free to check and comment in there. Thanks

novicecpp commented 11 months ago

I have a question for you now: would it then be better to also move HTTPRequests to ClientUtilities ? Since it is not used by CRABRest only.

Maybe move HTTPRequests to utils and move DBSREST to new file same level as CrabRestInterface.py?

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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

belforte commented 11 months ago

Hope I addressed all coments (only the uri vs. api confusion is left, but I think it is too much work). Will run Jenkins again. New code tagged as testSetDsetFilesStatus.2 Jenkinks OK

cmsdmwmbot commented 11 months ago

Jenkins results:

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

belforte commented 11 months ago

code with last changes is now tagged as testSetDsetFilesStatus.2 in the ad-hoc test branch

cmsdmwmbot commented 11 months ago

Jenkins results:

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

belforte commented 11 months ago

ooopppss. I had added content-type in the wrong place. But of course CRAB Server REST happily ignores it and everything kept working, but better not confuse the reader.

belforte commented 11 months ago

If there are no more objection I will do a BIG SQUASH and merge.

cmsdmwmbot commented 11 months ago

Jenkins results:

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

belforte commented 11 months ago

thanks @mapellidario and @novicecpp for review and advice. I will merge. And we can discuss separately if/how to add --recursive. Maybe wait for someone to ask for it :grin: ?

We still have time to decide if to keep old semantic (Invalidating a dataset will automatically invalidate all the files in the dataset) or leave it to user with two actions as I indicated above. It seems a soft decisions, with arguments in pro/con either choice.

belforte commented 11 months ago

Since in current implementation crab setdatasetstatus does not change status of individual files, I will print a note about this when the command is executed !

mapellidario commented 11 months ago

Thanks Stefano for the detailed description of dataset and file validity in DBS. Sorry for the dumb questions!

Do we already have an entry in the FAQs about DBS dataset and file status? Since we are allowing users to change it, we should also provide clear instructions about what those are and what the commands we provide do (or plan to do).

belforte commented 11 months ago

Dario,your questions were not dumb. The problem is that there is no real rule on how to use those status flags, DBS allows to set, then everybody can do as they see fit. I think Production changed their practice over time.

As user documentation this is what we have: https://twiki.cern.ch/twiki/bin/view/CMSPublic/CRAB3FAQ#Can_I_delete_a_dataset_I_publish it clearly needs a rewriting ! That is my next step.

cmsdmwmbot commented 11 months ago

Jenkins results:

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

cmsdmwmbot commented 11 months ago

Jenkins results:

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