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

edmPickEvents improvements #37208

Merged
merged 3 commits into from Mar 15, 2022
Merged

edmPickEvents improvements #37208

merged 3 commits into from Mar 15, 2022

Conversation

iarspider
Copy link
Contributor

PR description:

dasgoclient doesn't print all errors to stderr, some go to stdout. Since the errors are not proper JSON, edmPickEvents script will raise JSONDecodeError, but will not show the actual error. The following changes were added to get a better error message:

  1. An early detection of failure (test if exit_code != 0)
  2. A failsafe (catching JSONDecodeError)

PR validation:

Ran modified code locally.
Example command: edmPickEvents.py Cosmics/Run2011A-v1/RAW 160960:277:10001082,160960:277:10001058,160960:277:10001650

Example output from original code:

Traceback (most recent call last):
  File "/build/razumov2/CMSSW_12_3_X_2022-03-10-2300/./edmPickEvents.py", line 334, in <module>
    eventFiles = getFileNames(event, options.das_cli)
  File "/build/razumov2/CMSSW_12_3_X_2022-03-10-2300/./edmPickEvents.py", line 95, in getFileNames
    return getFileNames_dasgoclient(event)
  File "/build/razumov2/CMSSW_12_3_X_2022-03-10-2300/./edmPickEvents.py", line 134, in getFileNames_dasgoclient
    for row in json.loads(res):
  File "/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/external/python3/3.9.6-67e5cf5b4952101922f1d4c8474baa39/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/external/python3/3.9.6-67e5cf5b4952101922f1d4c8474baa39/lib/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc10/external/python3/3.9.6-67e5cf5b4952101922f1d4c8474baa39/lib/python3.9/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Example output with proposed changes:

DAS query error: return code 18
--- Standard output---
Validation error: unmatched dataset pattern

--- Standard error---

--- End ---

dasgoclient doesn't print all errors to stderr, some go to stdout. Since the errors are not proper JSON, edmPickEvents script will raise JSONDecodeError, but will not show the actual error. The following changes were added to get a better error message:
1) An early detection of failure (test if exit_code != 0) 
2) A failsafe (catching JSONDecodeError)
@iarspider
Copy link
Contributor Author

assign analysis

@smuzaffar
Copy link
Contributor

assign analysis

no need to explicitly assign the category for Pull requests, bot should do it automatically ( based on change set ) :-)

@iarspider
Copy link
Contributor Author

assign analysis

no need to explicitly assign the category for Pull requests, bot should do it automatically ( based on change set ) :-)

That makes sense :)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37208/28809

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • PhysicsTools/Utilities (analysis)

@cmsbuild, @santocch can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@qliphy
Copy link
Contributor

qliphy commented Mar 14, 2022

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa5464/23082/summary.html
COMMIT: ee6037b
CMSSW: CMSSW_12_4_X_2022-03-13-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37208/23082/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test_edmPickEvents had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3695377
  • DQMHistoTests: Total failures: 11
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3695344
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 204 log files, 45 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Mar 15, 2022

@iarspider It seems there is still error on edmPickEvents

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aa5464/23082/unitTests/log.txt
===== Test "test_edmPickEvents" ====

  • edmPickEvents.py /Cosmics/Run2011A-v1/RAW 160960:277:10001082,160960:277:10001058,160960:277:10001650

---> test test_edmPickEvents had ERRORS

@iarspider
Copy link
Contributor Author

iarspider commented Mar 15, 2022 via email

@qliphy
Copy link
Contributor

qliphy commented Mar 15, 2022

This PR does not fix an error, it merely makes actual error reported by dasgoclient visible.

Ok, thanks @iarspider

@perrotta
Copy link
Contributor

+1

  • As discussed in today's ORP and also explained above, this PR only makes the error more visible
  • The error message from the tests is now:
===== Test "test_edmPickEvents" ====
+ edmPickEvents.py /Cosmics/Run2011A-v1/RAW 160960:277:10001082,160960:277:10001058,160960:277:10001650

---> test test_edmPickEvents had ERRORS
TestTime:2
^^^^ End Test test_edmPickEvents ^^^^

@perrotta
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 84df093 into cms-sw:master Mar 15, 2022
@santocch
Copy link

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing).

@iarspider iarspider deleted the patch-4 branch February 27, 2024 10:16
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.

None yet

6 participants