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

11891 create mock classdata for mspileup #11905

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

d-ylee
Copy link
Contributor

@d-ylee d-ylee commented Feb 19, 2024

Fixes #11891

Status

In development

Description

Mocks the MSPileupUtils.py getPileupDocs call for testing via an MSPileup Emulator

Is it backward compatible (if not, which system it affects?)

YES

Related PRs

#11870
#11879

External dependencies / deployment changes

No

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 1 tests no longer failing
  • Python3 Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 2 warnings
    • 9 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded
    • 4 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14886/artifact/artifacts/PullRequestReport.html

@amaltaro
Copy link
Contributor

@d-ylee Dennis, shouldn't this have fixed those 3 unit tests mentioned in the GH issue?

@d-ylee
Copy link
Contributor Author

d-ylee commented Feb 19, 2024

@amaltaro Should I be replacing the MockRucio calls in those three tests to use MSPileup instead?

@d-ylee d-ylee force-pushed the 11891-create-mock-classdata-for-mspileup branch from 8f7c0b7 to 6999d0d Compare February 19, 2024 23:11
@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 tests no longer failing
    • 1 changes in unstable tests
  • Python3 Pylint check: failed
    • 2 warnings
    • 6 comments to review
  • Pylint py3k check: failed
    • 1 errors and warnings that should be fixed
  • Pycodestyle check: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14887/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
    • 1 tests no longer failing
  • Python3 Pylint check: failed
    • 2 warnings
    • 6 comments to review
  • Pylint py3k check: failed
    • 1 errors and warnings that should be fixed
  • Pycodestyle check: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14888/artifact/artifacts/PullRequestReport.html

@amaltaro
Copy link
Contributor

@d-ylee Dennis, we need to ensure that those 3 unit tests are using a patched (mocked) MSPileup instance/data.

For instance, this testGlobalBlockSplitting unit test:
https://github.com/dmwm/WMCore/blob/master/test/python/WMCore_t/WorkQueue_t/WorkQueue_t.py#L738

calls the WorkQueue updateLocationInfo method:
https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/WorkQueue/WorkQueue.py#L774

which in turn executes the DataLocationMapper class:
https://github.com/dmwm/WMCore/blob/master/src/python/WMCore/WorkQueue/DataLocationMapper.py#L71

Assuming that we are testing a workqueue element with pileup data, it means that DataLocationMapper will try to resolve the pileup location through MSPileup.

Hence, we need to patch MSPileup such that it uses mocked data - or uses the mock class. For that, you will have to run that unit test, check what is the value it is trying to look up at MSPileup, and prepare either the mock class or mocked data (MSPileup) to return the data expected by the unit test.

If you prefer to have a chat on this tomorrow, just ping me on mattermost please.

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 1 tests no longer failing
    • 2 changes in unstable tests
  • Python3 Pylint check: failed
    • 2 warnings
    • 6 comments to review
  • Pylint py3k check: failed
    • 1 errors and warnings that should be fixed
  • Pycodestyle check: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14889/artifact/artifacts/PullRequestReport.html

@d-ylee
Copy link
Contributor Author

d-ylee commented Feb 20, 2024

test this please

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 2 tests no longer failing
    • 2 changes in unstable tests
  • Python3 Pylint check: failed
    • 2 warnings
    • 6 comments to review
  • Pylint py3k check: failed
    • 1 errors and warnings that should be fixed
  • Pycodestyle check: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14895/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 2 tests no longer failing
    • 2 changes in unstable tests
  • Python3 Pylint check: failed
    • 10 warnings and errors that must be fixed
    • 3 warnings
    • 61 comments to review
  • Pylint py3k check: failed
    • 1 errors and warnings that should be fixed
  • Pycodestyle check: succeeded
    • 6 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14896/artifact/artifacts/PullRequestReport.html

@d-ylee
Copy link
Contributor Author

d-ylee commented Feb 20, 2024

@amaltaro Looking through Jenkins, it seems that the 3 tests in the Issue are passing and using the mocked MSPileup getPileupDocs:

WMCore_t.WorkQueue_t.WorkQueue_t.WorkQueueTest:testProcessingWithPileup
https://cmssdt.cern.ch/dmwm-jenkins/job/DMWM-WMCore-PR-test/14896/testReport/WMCore_t.WorkQueue_t.WorkQueue_t/WorkQueueTest/testProcessingWithPileup/

WMCore_t.WorkQueue_t.Policy_t.Start_t.Block_t.BlockTestCase:testPileupData
https://cmssdt.cern.ch/dmwm-jenkins/job/DMWM-WMCore-PR-test/14896/testReport/WMCore_t.WorkQueue_t.Policy_t.Start_t.Block_t/BlockTestCase/testPileupData/

WMCore_t.WorkQueue_t.WorkQueue_t.WorkQueueTest:testPileupOnProduction
https://cmssdt.cern.ch/dmwm-jenkins/job/DMWM-WMCore-PR-test/14896/testReport/WMCore_t.WorkQueue_t.WorkQueue_t/WorkQueueTest/testPileupOnProduction/

@amaltaro
Copy link
Contributor

Cool! I see you also fixed a log record and I would like to actually get it deployed in production tomorrow.
Can you please squash these commits? Either in 2 commits as usual, or in 3 (src changes, src emulator changes, test changes). Thanks @d-ylee

@amaltaro
Copy link
Contributor

Dennis, you will need to rebase this pull request because I merged the url variable fix, see: #11908 , sorry about that.

@d-ylee d-ylee force-pushed the 11891-create-mock-classdata-for-mspileup branch from f27331a to 09d4398 Compare February 21, 2024 15:12
Patch locations where getPileupDocs is used

Add proper RSE sites in MockMSPileupAPI
Use MSPileupObj schema() instead of a predefined dict
Removed ThisBlock from Block_t
@d-ylee d-ylee force-pushed the 11891-create-mock-classdata-for-mspileup branch from 09d4398 to 7a18a9f Compare February 21, 2024 15:12
@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: succeeded
    • 2 tests no longer failing
    • 3 changes in unstable tests
  • Python3 Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 36 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded
    • 2 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/14904/artifact/artifacts/PullRequestReport.html

Copy link
Contributor

@amaltaro amaltaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Dennis! I left a couple of comments for future us, but this is good to go in.

return
if self.mockMSPileup:
self.msPileupPatchers = []
patchMSPileupAt = ['WMCore.MicroService.MSTransferor.MSTransferor.getPileupDocs', # TODO: Need to use Services.MSPileupUtils
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dennis, can you please create a new GH issue such that we don't forget to adopt the same MSPileup library across the project - and also to deprecate the one under Tools.Common module? Thanks


queryDict = queryDict or {}

pdict = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as data grows we might consider making a json dump for this.

@amaltaro amaltaro merged commit 52ae759 into dmwm:master Feb 21, 2024
3 of 4 checks passed
@d-ylee d-ylee deleted the 11891-create-mock-classdata-for-mspileup branch February 22, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create mock class/data for MSPileup
3 participants