Check CMSSW release before getting skim-datatier map #11110
Conversation
|
Jenkins results:
|
|
The Pylint --py3k check fails because of a preexisting problem. I'm not sure what to do in this case @amaltaro @todor-ivanov |
germanfgv
changed the title
germanfgv
changed the title
There was a problem hiding this comment.
Thanks for providig this fix @germanfgv. It looks good and I think estimating if PY3 code is supported by cutting the set of CMSSW version in two slices ( e.g. starting from CMSSW_10_3_0) is a good way of proceeding here. I just wanted to ask a general question, because I am not completely sure if and how to estimate the version trough which we should draw the line. Did you try with multiple CMSSW versions? Because one thing is if with this version that particular call has been fixed, something totally different would be the release which provides full PY3 compatibility. BTW, I think this information is retrievable from other sources, no need for trail and error method.
| @@ -160,7 +161,7 @@ def skimToDataTier(cmsswVersion, skim): | |||
| command += "cd %s\n" % scramBaseDirs[0] | |||
| command += "eval `scramv1 runtime -sh`\n" | |||
|
|
|||
| if PY3: | |||
| if isCMSSWSupported(cmsswVersion, "CMSSW_10_3_0"): | |||
There was a problem hiding this comment.
Is this the exact version from which on this getSkimDatatier method has been fixed to support python3?
There was a problem hiding this comment.
Hi Todor. What I did is I looked for the PR that made the changes: cms-sw/cmssw#23932
It was introduced in CMSSW_10_3_0_pre1, but I think isCMSSWSupported() does not handle pre-releases.
There was a problem hiding this comment.
Hi @germanfgv I think this check is pretty much enough for the current situation. Thanks!
There was a problem hiding this comment.
That's correct, this method doesn't go beyond the 3rd digit of the release number.
Thanks for this confirmation, German, I missed your in-thread reply :)
There was a problem hiding this comment.
The Pylint --py3k check fails because of a preexisting problem.
pre-existing issues are not a problem, you are welcome to fix those if it's straight forward though.
German, these changes look good to me. However, I'd rather wait for your answer to Todor's question. Thanks
| @@ -160,7 +161,7 @@ def skimToDataTier(cmsswVersion, skim): | |||
| command += "cd %s\n" % scramBaseDirs[0] | |||
| command += "eval `scramv1 runtime -sh`\n" | |||
|
|
|||
| if PY3: | |||
| if isCMSSWSupported(cmsswVersion, "CMSSW_10_3_0"): | |||
There was a problem hiding this comment.
Hi @germanfgv I think this check is pretty much enough for the current situation. Thanks!
Fixes #11109
Status
ready
Description
Uses
isCMSSWSupported()to determine the Python version to use to get skim's datatier, instead of just using the agent's python version.Is it backward compatible (if not, which system it affects?)
YES
Related PRs
None
External dependencies / deployment changes
None