First attempt to fix DQM GUI urls in RelMon #27804
Conversation
|
please test |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27804/11509
|
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @jfernan2 for master. It involves the following packages: Utilities/RelMon @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
I wonder why "Utilities/RelMon" is in the "reconstruction" category instead of "dqm": just a mistake or is there any reason behind? |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@smuzaffar @mrodozov |
|
@slava77 testing the python files could be generalize up to "if they compile" (and they does in this case) but for the "what they produce when run" I think a unit test will be required (if there is none, I don't know). Assuming there is no test the proper way to test it would be to ask @jfernan2 what was used to local test it, add it as a unit test and rerun. Apart from that, the python part just ... compiles. If it didn't the scram command would've failed. |
|
@mrodozov in my test I just made sure that the piece of python code I am modifying/introducing was working locally (with a print) as expected, producing the desired string for the url link. Not a real unit test. |
|
@jfernan2 : I think that this code, and the whole "Utilities/RelMon" package, should belong to dqm rather than reconstruction. If you agree with it, I could just switch "Utilities/RelMon" to dqm in the categories map. Or alternatively, we can can at least share it if you think there are reasons for having it in reco that I am not aware of. |
|
assign dqm |
|
New categories assigned: dqm @jfernan2,@andrius-k,@schneiml,@fioriNTU,@kmaeshima you have been requested to review this Pull request/Issue and eventually sign? Thanks |
Just to be sure, what I meant is that the code updated here is used by cms-bot in the run-pr-comparisons jobs |
|
@perrotta fine by us (DQM). I do not know why it is under reco umbrella (historical reasons?), I just learnt it was this way when creating this PR |
it's not exactly a unit test to test a "unit" but that's not unusual to test pieces like this when required. Example is this: |
|
Thak you @jfernan2 : cms-bot PR cms-sw/cms-bot#1190 makes the switch |
|
+1
|
|
I am sorry @mrodozov but I am not able to create a test file like you propose which mimics the RelMon environment. |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
|
@jfernan2 @perrotta @slava77 we already discussed this point in the recent past, as far as I know RelMon is historically under the reconstruction umbrella because it was originally developed by @dpiparo working with @VinInn to monitor and study workflows and in particular RECO performances. But strictly speaking is more a DQM tool than a RECO one IMO |
PR description:
This PR tries to fix issue #27800
Links points to root Everything workspace instead of dedicated folder to simplify
PR validation:
Tested locally
if this PR is a backport please specify the original PR:
No backport