Skip to content
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

questions on conddb behaviour with "read-only" CMSSW areas #42292

Closed
missirol opened this issue Jul 18, 2023 · 12 comments · Fixed by #42324
Closed

questions on conddb behaviour with "read-only" CMSSW areas #42292

missirol opened this issue Jul 18, 2023 · 12 comments · Fixed by #42324

Comments

@missirol
Copy link
Contributor

I'm trying to understand better the behavior of conddb in a not-so-standard situation.

  • The HLT group uses a few nodes in the CMS online network for testing ("Hilton" nodes). SysAdmins install CMSSW releases on theses nodes whenever this is requested.
  • When running tests on these nodes, HLT currently activates a certain CMSSW release without creating a local area (meaning, without using cmsrel, but just running cmsenv where the centrally-installed release is).

The test command in question is in [1]. The issue is that this works when using a full release, but it does not work when using a patch release [2]. The error seen with patch release comes from here, and has to do with how the script tries to find the file pluginUtilities_payload2xml.so.

  • Full-release case: [1] works, the file pluginUtilities_payload2xml.so is found under ${CMSSW_BASE}/lib/${SCRAM_ARCH}. The environment variables are as follows

    CMSSW_BASE=/opt/offline/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_10
    CMSSW_RELEASE_BASE=
    CMSSW_FULL_RELEASE_BASE=
  • Patch-release case: [1] fails with the error in [2]. The file pluginUtilities_payload2xml.so is available under ${CMSSW_FULL_RELEASE_BASE}/lib/${SCRAM_ARCH}, but conddb throws an error because it does not find it in ${CMSSW_BASE}/lib/${SCRAM_ARCH} and ${CMSSW_RELEASE_BASE} is empty (see logic here). The environment variables are as follows

    CMSSW_BASE=/opt/offline/el8_amd64_gcc11/cms/cmssw-patch/CMSSW_13_0_10_patch1
    CMSSW_RELEASE_BASE=
    CMSSW_FULL_RELEASE_BASE=/opt/offline/el8_amd64_gcc11/cms/cmssw/CMSSW_13_0_10

The only workaround I could find is below, and it makes [1] work with patch releases (at least in my specific use case).

CMSSW_RELEASE_BASE=${CMSSW_BASE} conddb [...]

Questions:

  • Is the workaround acceptable, or can it somehow have unwanted consequences ?
  • Could the logic here be improved such that no workaround is needed ?
  • In general, is it okay to activate a release this way (from where it was centrally installed), or is it recommended to always create a local area with cmsrel ?

[1]

conddb --db "oracle+frontier://@frontier%3A%2F%2F%28proxyurl%3Dhttp%3A%2F%2Flocalhost%3A3128%29%28serverurl%3Dhttp%3A%2F%2Flocalhost%3A8000%2FFrontierOnProd%29/CMS_CONDITIONS" dump d79b67dce0450d5fa6ad5c9aae636a5d717392f7 > tmp.xml

[2] Error using [1] with a patch release:

[2023-07-18 07:22:54,576] INFO: Connecting to oracle+frontier://@frontier%3A%2F%2F%28proxyurl%3Dhttp%3A%2F%2Flocalhost%3A3128%29%28serverurl%3Dhttp%3A%2F%2Flocalhost%3A8000%2FFrontierOnProd%29/CMS_CONDITIONS [frontier://FrontierOnProd%29/CMS_CONDITIONS]
[2023-07-18 07:22:55,389] INFO: Found payload of type L1TUtmTriggerMenu
[2023-07-18 07:22:55,389] ERROR: No built-in library /opt/offline/el8_amd64_gcc11/cms/cmssw-patch/CMSSW_13_0_10_patch1/lib/el8_amd64_gcc11/pluginUtilities_payload2xml.so found with XML converters.
@cmsbuild
Copy link
Contributor

A new Issue was created by @missirol Marino Missiroli.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@francescobrivio
Copy link
Contributor

assign db

@cmsbuild
Copy link
Contributor

New categories assigned: db

@perrotta,@consuegs,@francescobrivio,@saumyaphor4252,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mmusich
Copy link
Contributor

mmusich commented Jul 18, 2023

looks to me that a fix could be as simple as this:

diff --git a/CondCore/Utilities/python/cond2xml.py b/CondCore/Utilities/python/cond2xml.py
index 1a64d662f45..7c6ea35c310 100644
--- a/CondCore/Utilities/python/cond2xml.py
+++ b/CondCore/Utilities/python/cond2xml.py
@@ -86,6 +86,10 @@ class CondXmlProcessor(object):
         devCheckout = (releaseBase != '')
         if not devCheckout:
             logging.debug('Looks like the current working environment is a read-only release')
+            if not os.path.exists( libPath ):
+                if "CMSSW_FULL_RELEASE_BASE" in os.environ:
+                    libDir = os.path.join( os.environ["CMSSW_FULL_RELEASE_BASE"], 'lib', os.environ["SCRAM_ARCH"] )
+                    libPath = os.path.join( libDir, libName )
         if not os.path.exists( libPath ) and devCheckout:
             # main release ( for dev checkouts )
             libDir = os.path.join( releaseBase, 'lib', os.environ["SCRAM_ARCH"] )

@tvami
Copy link
Contributor

tvami commented Jul 20, 2023

looks to me that a fix could be as simple as this:

So should this fix PRed? (Sorry if it was and I missed it)

@francescobrivio
Copy link
Contributor

looks to me that a fix could be as simple as this:

So should this fix PRed? (Sorry if it was and I missed it)

I will test it and do PR in the next days :D

@mmusich
Copy link
Contributor

mmusich commented Jul 20, 2023

So should this fix PRed? (Sorry if it was and I missed it)

for the moment I opened #42324, feel free to discard it (in my tests it worked).

@tvami
Copy link
Contributor

tvami commented Jul 20, 2023

Thanks Marco!

@missirol
Copy link
Contributor Author

Thanks @mmusich , I tested your suggestion and it works. If you or @cms-sw/db-l2 find it useful, I wrote missirol@ea229a8 while testing, but any solution works for me.

If possible, I would like to backport the fix to 13_0_X to remove workarounds in some HLT scripts outside CMSSW (I can take care of the backports if it helps).

@francescobrivio
Copy link
Contributor

Thanks @mmusich and @missirol for the nice (and quick!) fixes!
Marco, if you don't mind I'd go with Marino's commit that, despite being a bit longer (code lines wise), has a bit of restructuring and makes the code easier to read/understand (at least for me), is that ok?

@tvami
Copy link
Contributor

tvami commented Jul 25, 2023

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants