-
Notifications
You must be signed in to change notification settings - Fork 107
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
ad-hoc fix for failed workflow while creating WQEs #11810
Conversation
ef844ea
to
bfab120
Compare
Jenkins results:
|
@amaltaro , @todor-ivanov , @khurtado please review this ad-hoc patch. I added additional call to Rucio via |
Jenkins results:
|
apparently I broke unit tests with missing import, will fix it in a bit |
bfab120
to
baa8734
Compare
Jenkins results:
|
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.
thanks @vkuznet It looks good
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.
@vkuznet please find my comments along the code.
src/python/WMCore/WorkQueue/Policy/Start/StartPolicyInterface.py
Outdated
Show resolved
Hide resolved
src/python/WMCore/WorkQueue/Policy/Start/StartPolicyInterface.py
Outdated
Show resolved
Hide resolved
When the time to merge/backport/deploy it comes, the instructions are very similar to what was mentioned in this comment: Tagging and how to tag is described in this wiki: https://github.com/dmwm/WMCore/wiki/TaggingAndReleasing#new-tagging-convention |
baa8734
to
f7ccff0
Compare
Jenkins results:
|
f7ccff0
to
352b226
Compare
Jenkins results:
|
352b226
to
0919e66
Compare
Jenkins results:
|
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.
@vkuznet Valentin, as mentioned along the code, the getDatasetLocations()
function is not properly used in this PR. It looks like we only have 1 src/ and 1 test/ location calling that function and I'd be totally in favor of refactoring it to get a flat list of datasets (removing the dbs dependency, given that it is not used AT ALL during the location resolution).
src/python/WMCore/WorkQueue/Policy/Start/StartPolicyInterface.py
Outdated
Show resolved
Hide resolved
src/python/WMCore/WorkQueue/Policy/Start/StartPolicyInterface.py
Outdated
Show resolved
Hide resolved
test this please |
Jenkins results:
|
Jenkins results:
|
Jenkins results:
|
Alan, please review again, the unit test which fails (WMCore_t.Services_t.Rucio_t.RucioUtils_t.RucioUtilsTest:testWeightedChoice ) is not related to this PR. |
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.
Valentin, these changes look good to me. Can you please squash those commits accordingly? Thanks
6fdba82
to
ba8bc66
Compare
Alan, I squashed commits. |
Jenkins results:
|
Alan, please let me know if you'll merge this PR or want me to proceed, so far every PR I made was merged by you and I would like to avoid waiting deadlock if you assume otherwise for this specific PR. |
@vkuznet Valentin, please go ahead and backport this fix, tag and upgrade global workqueue in cmsweb-testbed. See this comment for further details and/or let me know if you have any questions. For now, I am going to upgrade testbed with the latest 2.2.6rc5 (being built at this very moment), but once you have a dedicated hot-fix for global workqueue available, just upgrade global workqueue again in testbed. |
Fixes #11784
Status
ready
Description
This is ad-hoc patch rather an actual fix for issue #11784. So far it only disables exception before create of WQE when there is no Input data placement present in a given spec (which may be the case when data is on tape). Instead we would like to have logging warnings to monitor and further understand the situation.
Is it backward compatible (if not, which system it affects?)
MAYBE
Related PRs
External dependencies / deployment changes