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

Support of plot-specific input paramters for Payload Inspector #28648

Merged

Conversation

ggovi
Copy link
Contributor

@ggovi ggovi commented Dec 18, 2019

PR description:

The developer level interface of the Payload Inspector has been extended to support plot-specific input parameters

PR validation:

The new functionality has been tested with the test options of the getPayloadData script
More validation is expected after the deployment on the development service

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28648/13205

  • This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28648/13227

  • This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28648/13229

  • This PR adds an extra 24KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6f5175/4072/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2813777
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2813435
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@fabiocos
Copy link
Contributor

@davidlange6 you have invested wor in the past to get rid of some boost dependencies within CMSSW, here we have a further addition of boost dependency (ok, the DB area is entirely dependent on boost...)
@smuzaffar @Dr15Jones @makortel Do we have a clear definition of a policy wrt boost?

@davidlange6
Copy link
Contributor

davidlange6 commented Dec 22, 2019 via email

@mmusich
Copy link
Contributor

mmusich commented Jan 6, 2020

Hello all,
in the light of the #28648 (comment) may I ask what is the foreseen trajectory of this PR?
It seems to me that more work is needed to move with the migration described above, and in the meanwhile this addition would solve the long standing issue CMSDBBROWS-82. Additionally there is already a new line of development based on the changes proposed here which is waiting this PR to be merged.

@silviodonato
Copy link
Contributor

@ggovi

@silviodonato
Copy link
Contributor

hold

@cmsbuild
Copy link
Contributor

Pull request has been put on hold by @silviodonato
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@mmusich
Copy link
Contributor

mmusich commented Jan 15, 2020

@silviodonato for my education what holds this PR?

@silviodonato
Copy link
Contributor

@mmusich @ggovi I was expecting a confirmation that this PR should go in without waiting for the migration to pybind11 ( #28648 (comment) )

@ggovi
Copy link
Contributor Author

ggovi commented Jan 15, 2020

@silviodonato I'm not sure who is supposed to provide this confirmation. Fabio/David?

@silviodonato
Copy link
Contributor

As far as I understood @davidlange6 agreed that the migration is complicated and should be made in a different PR. So I think we can proceed with merging this PR in the next IB.

@silviodonato
Copy link
Contributor

unhold

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

@silviodonato
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.

None yet

6 participants