-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DQM] Added a new test for ROOT file uploads to DQMGUI #46551
Conversation
cms-bot internal usage |
please test |
original_file_size=$(stat --printf="%s" $OLD_FILENAME) | ||
|
||
# TODO: Is there any way to not hardcode the required test name, TestDQMServicesDemo? | ||
UNIT_TESTS_BASE_PATH=${LOCALTOP}/unit_tests/TestDQMServicesDemo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nothingface0 , looks like this is not used , so do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm cleaning up, thanks
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46551/42431 |
A new Pull Request was created by @nothingface0 for master. It involves the following packages:
@antoniovagnerini, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
visDQMUpload.py "$DEV_DQMGUI_URL" $NEW_FILENAME | ||
|
||
# Wait for a bit for the file to be imported into the DQMGUI's index. | ||
SECONDS_TO_WAIT=60 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if file import is not immediate then I would suggest to increase the max wait to 600s
i.e 10 mins and may be change the sleep to 10 sec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, better safe than sorry.
f147f1b
to
08fc388
Compare
08fc388
to
bde61fd
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46551/42432 |
Pull request #46551 was updated. @antoniovagnerini, @cmsbuild, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please check and sign again. |
bde61fd
to
97702b3
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46551/42459 |
Pull request #46551 was updated. @antoniovagnerini, @cmsbuild, @rseidita can you please check and sign again. |
Would it make sense to move the verification of the compatibility of the ROOT file directly upon reception? |
From my side, it sounds like a good idea, but in practice, I don't know how long or intensive these specific checks are. It's generally good practice to avoid directly exposing long and intensive operations under a webserver's endpoint, as this could lead to hangs. I could verify how intensive those checks are, and if they're "lightweight", I could move the checks directly in the DQMGUI's |
please test |
-1 Failed Tests: RelVals-INPUT
RelVals-INPUT
Expand to see more relval errors ...
Comparison SummarySummary:
|
+1 Size: This PR adds an extra 12KB to repository Comparison SummarySummary:
|
+1
|
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. @rappoccio, @antoniovilela, @mandrenguyen, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Added a test that uses a ROOT file generated by the TestDQMServicesDemo test and uploads it to the dev DQMGUI. This should cover the case where CMSSW moves to a new ROOT release, which might be incompatible with the one being used at the DQMGUI.
Resolves: #43590
Thanks to @smuzaffar for the help.
PR validation:
Tests run normally in LXPLUS.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Not a backport.