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

TkAl: fix PV validation and PV resolution submission scripts #44467

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Mar 19, 2024

PR description:

The goal of this PR is to fix the scripts submitPVResolutionJobs.py and submitPVValidationJobs.py such that they can run in recent releases and with recent (Run 3) data.
The related unit tests are improved such that, in addition to technically creating the configuration to be run with HTCondor, said configuration is also tested for at least one job (running locally).

PR validation:

Run successfully:

  • scram b runtests_SubmitPVsplit
  • scram b runtests_SubmitPVrbr

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, but it will be backported to CMSSW_14_0_X for 2024 data-taking purposes.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 19, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44467/39551

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich for master.

It involves the following packages:

  • Alignment/OfflineValidation (alca)

@perrotta, @consuegs, @cmsbuild, @saumyaphor4252 can you please review it and eventually sign? Thanks.
@mmusich, @tlampen, @yuanchao, @tocheng, @rsreds, @adewit this is something you requested to watch as well.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor Author

mmusich commented Mar 19, 2024

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44467/39557

@cmsbuild
Copy link
Contributor

Pull request #44467 was updated. @perrotta, @saumyaphor4252, @consuegs, @cmsbuild can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Mar 19, 2024

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dbc1c1/38270/summary.html
COMMIT: 8687f99
CMSSW: CMSSW_14_1_X_2024-03-19-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44467/38270/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 1 errors in the following unit tests:

---> test SubmitPVsplit had ERRORS

Comparison Summary

Summary:

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44467/39568

@cmsbuild
Copy link
Contributor

Pull request #44467 was updated. @cmsbuild, @saumyaphor4252, @consuegs, @perrotta can you please check and sign again.

@mmusich
Copy link
Contributor Author

mmusich commented Mar 20, 2024

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dbc1c1/38276/summary.html
COMMIT: 6f66382
CMSSW: CMSSW_14_1_X_2024-03-19-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44467/38276/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@perrotta
Copy link
Contributor

+1

  • The updated unit test succeeds

@cmsbuild
Copy link
Contributor

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. @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit dd3cc75 into cms-sw:master Mar 20, 2024
11 checks passed
@mmusich mmusich deleted the mm_submitPVValidationJobs_updates branch March 20, 2024 17:43
@aandvalenzuela
Copy link
Contributor

Hello @mmusich,

I noticed both unit tests SubmitPVsplit and SubmitPVrbr fail in IBs when the number of runs is 0. It has happened two times:

In both cases, the error appears when trying to copy a file that I think is only generated if there is at least one run:

Will run on 1 workflows
>>>> This is Data!
>>>> Doing run based selection
Will run on  0 runs: 
 []
first run: 1 last run: 999999


======================================================================
|| WARNING: won't run on any run, probably DAS returned an empty query,
|| but that's fine because this is a unit test!
======================================================================




 TESTING Primary Vertex Validation script execution ...
cp: cannot stat './BASH/PVValidation_testingOfflineGT_HLTPhysics_Run2023D_0.sh': No such file or directory
/data/cmsbld/jenkins/workspace/ib-run-qa/CMSSW_14_1_MULTIARCHS_X_2024-03-31-2300/src/Alignment/OfflineValidation/test/testingScripts/test_unitSubmitPVrbr.sh: line 28: ./PVValidation_testingOfflineGT_HLTPhysics_Run2023D_0.sh: No such file or directory
Failure running PVValidation script: status 127

@mmusich
Copy link
Contributor Author

mmusich commented Apr 2, 2024

@aandvalenzuela I think that's fine - that's only a symptom. It should be investigated instead why the das query was empty.

@aandvalenzuela
Copy link
Contributor

Thanks @mmusich, I thought it was fine to have an empty query from DAS (as reported in the warning message) and that it should be handled by the unit test. In any case, I am trying to see if there is a way to prevent DAS from returning an empty query.

@mmusich
Copy link
Contributor Author

mmusich commented Apr 2, 2024

@aandvalenzuela

thought it was fine to have an empty query from DAS (as reported in the warning message) and that it should be handled by the unit test.

It's fine in the sense that when I created the test, since DAS query failures are fairly frequent in IB tests (for reasons not clear to me), I thought of avoiding the test to fail every time. In any case here's a patch for that #44588. Still it would be nice to understand why the query failed specifically for MULTIARCHS and not elsewhere.

@smuzaffar
Copy link
Contributor

@aandvalenzuela @mmusich , I think this das error has nothing to do with these unit tests. Normally when the voms proxy is in bad state [a] then all unit tests which run das queries fail. We had tried to fix this by creating a dedicated proxy for each jenkins job but still do not understand why proxy file is either deleted or overridden

[a]

2024/04/01 10:40:46 ERROR failed to parse X509 proxy: crypto/tls: failed to parse key: asn1: syntax error: data truncated

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

Successfully merging this pull request may close these issues.

6 participants