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

Correct url of the GEN fragment in a Pythia8Interface test and of the RelMon main page, after SSO migration #43093

Conversation

perrotta
Copy link
Contributor

@perrotta perrotta commented Oct 24, 2023

PR description:

Unit test "TestGeneratorInterfacePythia8ConcurrentGeneratorFilter" was failing in the IBs because of a modified address in the pdmv repository of the GEN fragment needed for the test:
From: cms-pdmv.cern.ch
To: cms-pdmv-prod.web.cern.ch

The change in the url of pdmv scripts, to access McM, get in with recent SSO migration

Since I found another such case in Utilities/RelMon/python/definitions.py, I decided to fix also the url listed there in this PR (title and description modified from the original PR)

PR validation:

scram b runtests works without errors

Backports

The unit test is currently failing in all releases: if we want to fix it, this simple fix should be backported in all them

@perrotta
Copy link
Contributor Author

perrotta commented Oct 24, 2023

assign pdmv

@cms-sw/pdmv-l2 the unit test error showed up first in CMSSW_13_3_X_2023-10-22-2300, while it was not present in the previous days IBs: did you move your staffs sometimes on Sunday?

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43093/37331

@cmsbuild
Copy link
Contributor

New categories assigned: pdmv

@AdrianoDee,@sunilUIET,@miquork you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @perrotta (Andrea Perrotta) for master.

It involves the following packages:

  • GeneratorInterface/Pythia8Interface (generators)

@GurpreetSinghChahal, @mkirsano, @cmsbuild, @SiewYan, @sunilUIET, @alberto-sanchez, @menglu21, @miquork, @bbilin, @AdrianoDee can you please review it and eventually sign? Thanks.
@mkirsano, @alberto-sanchez this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor Author

please test

@perrotta
Copy link
Contributor Author

perrotta commented Oct 24, 2023

type bug

@iarspider
Copy link
Contributor

@perrotta I made a PR with slightly different fix for the same issue - #43089, should I close it in favor of this?

@perrotta
Copy link
Contributor Author

@perrotta I made a PR with slightly different fix for the same issue - #43089, should I close it in favor of this?

Ah, sorry @iarspider I did not notice it, while I should have checked...
Let @cms-sw/pdmv-l2 decide which is the most suitable fix. Probably one could mix them: if the move is supposed to be permanent, I think that the correct address should be used. On the other hand, one could also allow following redirections, to prevent further moves of the same url

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bfe9cc/35368/summary.html
COMMIT: 49bd787
CMSSW: CMSSW_13_3_X_2023-10-23-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43093/35368/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • Reco comparison results: 30 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3357400
  • DQMHistoTests: Total failures: 1070
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3356308
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@sunilUIET
Copy link
Contributor

Hi @perrotta

This change, to access McM, get in with recent SSO migration

https://cms-talk.web.cern.ch/t/pdmv-mcm-migration-to-the-new-sso/30378

So, we will need to update the pdmv url in scripts.

@perrotta
Copy link
Contributor Author

Thank you @sunilUIET
I will rebase this PR on tomorrow's IB, then

@perrotta perrotta force-pushed the FixUnitTestInGeneratorInterfacePythia8Interface branch from 49bd787 to a7ea6fd Compare October 25, 2023 08:42
@perrotta
Copy link
Contributor Author

please test

@sunilUIET
Copy link
Contributor

+pdmv

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bfe9cc/35402/summary.html
COMMIT: a7ea6fd
CMSSW: CMSSW_13_3_X_2023-10-24-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43093/35402/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 76 lines to the logs
  • Reco comparison results: 33 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3351458
  • DQMHistoTests: Total failures: 1076
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3350360
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 166 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor Author

@cms-sw/generators-l2 @cms-sw/dqm-l2 : this is rather trivial, and already approved by pdmv. Could you please have a look?

@tjavaid
Copy link

tjavaid commented Oct 31, 2023

+1

@menglu21
Copy link
Contributor

+1

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

@antoniovilela
Copy link
Contributor

+1

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.

7 participants