-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add support for wildcard in EDAlias type field #31269
Conversation
Also add a shorthand EDAlias.allProducts()
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31269/17950
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: FWCore/Framework @makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
process.intalias = cms.EDAlias( | ||
intprod = cms.EDAlias.allProducts(), | ||
# can't use allProducts() because the product instance '' would lead to duplicate brances to be aliased | ||
intprod2 = cms.VPSet( | ||
cms.PSet(type = cms.string('*'), fromProductInstance = cms.string('foo2')), | ||
cms.PSet(type = cms.string('*'), fromProductInstance = cms.string('another2')), | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that
alias = cms.EDAlias(
module = cms.VPSet(
cms.PSet(
type = cms.string("*")
)
)
)
and the allProducts()
shorthand work only if a module does not produce multiple collections with the same type ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that ... and the
allProducts()
shorthand work only if a module does not produce multiple collections with the same type ?
It does not. The first added test
cmssw/FWCore/Integration/test/EDAlias_t.cpp
Line 136 in c0144f2
process.intalias = cms.EDAlias(intprod = cms.EDAlias.allProducts()) |
tests exactly that, creating an EDAlias for a module that produces three
edmtest::IntProduct
collections.
This test tests a case where products from two modules are aliased-for in a single EDAlias, and that type type wildcard works there as well. Because of my laziness of constructing the test (essentially the intprod
and intprod2
producing products of the same type, and both producing a product without product instance name) the simpler
process.intalias = cms.EDAlias(
intprod = cms.EDAlias.allProducts(),
intprod2 = cms.EDAlias.allProducts(),
)
would lead to two products of (edmtest::IntProduct, "intalias", "", "TEST")
, which is not allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, the conflict would be between two modules producing a collection with the same type and label.
@wddgit Could you review the changes? Thanks. |
Yes. I will review it today. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31269/18104
|
Pull request #31269 was updated. @makortel, @smuzaffar, @cmsbuild, @Dr15Jones can you please check and sign again. |
@cmsbuild, please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
This PR adds support for wildcard in EDAlias type field, i.e.
For the case that the EDAlias should pick all products of a module, the following shorthand is included
Resolves #29847
PR validation:
Framework unit tests pass