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

L1T2016 is completely transported to L1T #18352

Merged
merged 2 commits into from Apr 19, 2017

Conversation

EsmaeelEskandari
Copy link
Contributor

No description provided.

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DQM/L1TMonitorClient

@cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@kreczko this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@EsmaeelEskandari EsmaeelEskandari changed the title L1T2016 is completely tranported to L1T L1T2016 is completely transported to L1T Apr 13, 2017
@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 19, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19244/console Started: 2017/04/19 13:03

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3296 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1826273
  • DQMHistoTests: Total failures: 59955
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1766145
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@threus
Copy link
Contributor

threus commented Apr 25, 2017

@EsmaeelEskandari @thomreis is there a 90X back port of this PR?

@thomreis
Copy link
Contributor

@EsmaeelEskandari please confirm that this is also included in #18043 . The commit messages differ.

@EsmaeelEskandari
Copy link
Contributor Author

EsmaeelEskandari commented Apr 26, 2017

@thomreis : This PR includes just the last commit of #18043
06f7868
not all of its commits.

@thomreis
Copy link
Contributor

@EsmaeelEskandari OK, so this together with #17818 and #18352 gives us the complete set that is also in 91x already.

@threus
Copy link
Contributor

threus commented Apr 26, 2017

@EsmaeelEskandari @thomreis thanks for the info. Do I understand correctly, that the remaining commit of this PR: 9bd7a42 is not back ported to 90X yet? If so, could you please back port it if you need it? The point is, we merged (by our mistake) this PR in the online DQM, which is running on 90X, not 91X, so we need a 90X back port. Thanks and sorry for the inconvenience.

@EsmaeelEskandari
Copy link
Contributor Author

@threus @thomreis : Sorry, I am a little bit confused. I don't understand when PR #18352 is merged now what is the difference between its commits, one of them is back ported and one of them is not!
I should clarify that I made three PRs: #17818 #18043 and #18352. PRs #17818 and #18352 are merged with 91X. We just need #18043 to be merged.
Also PRs #18043 and #18352 have one common commit 06f7868.
Is it possible PR #18043 to be mereged now or I should drop the common commit?

@threus
Copy link
Contributor

threus commented Apr 26, 2017

@EsmaeelEskandari, don't worry, please keep everything as it is now :) sorry, maybe I was not clear.
I only wanted to know if this PR is fully back ported to 90X, because we (DQM) will have to drop this PR from online DQM, which is running CMSSW_9_0_2 and I was looking for the 90X version of this PR.
Maybe easiest is to call me on Skype if you want (tomas.hreus)..

@thomreis
Copy link
Contributor

thomreis commented Apr 26, 2017

@EsmaeelEskandari I think there is indeed no backport for this commit 9bd7a42

#18043 does not contain the changes made in that commit.

So I believe we need an additional PR to 90x containing only 9bd7a42 to be complete and in sync with the 91x PRs.

@EsmaeelEskandari
Copy link
Contributor Author

@threus : So please let me know about any problem in merging #18043?

@thomreis
Copy link
Contributor

@EsmaeelEskandari if the accidentally merged (at P5) PR #18352 is removed from the online DQM system and only #18043 is merged the L1T2016 directory will reappear again since the commit 9bd7a42 is not included in #18043

Therefore we need an additional PR to 90x with only the commit 9bd7a42

@threus
Copy link
Contributor

threus commented Apr 26, 2017

@thomreis exactly.
@EsmaeelEskandari you can see the result without PR #18352 in the playback GUI: https://cmsweb.cern.ch/dqm/online-playback

@thomreis
Copy link
Contributor

The missing commit is in #18474 that I just made.

@threus
Copy link
Contributor

threus commented Apr 26, 2017

great, thanks!

@EsmaeelEskandari
Copy link
Contributor Author

@thomreis : Thanks for your help.
@threus : Thank you too. I am seeing https://cmsweb.cern.ch/dqm/online-playback, as Thomas said the L1T2016 is re-appeared which by deploying PR #18474, this issue will be solved and everything will be fine.

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