<div dir="ltr">Hey gang,<div><br></div><div>In the process of fixing issue <a href="https://pulp.plan.io/issues/7282">7282</a>, I have encountered a behavior from advisory-merge that appears to be the deliberately-desired behavior - and I'm not sure it's really what we want to do? I need some thoughts on this. Figured I'd start in email, because it's advisory-merge, and complicated, and trying to follow a verbal description is confusing. More confusing.</div><div><br></div><div>The PR with the proposed fix is <a href="https://github.com/pulp/pulp_rpm/pull/1973">https://github.com/pulp/pulp_rpm/pull/1973</a></div><div><br></div><div>The scenario is as follows:</div><div><br></div><div>1) Upload an advisory that has a package-list with one RPM, say 'bear', to a repository.</div><div>2) Upload an *identical* advisory, except for having a different package list with one RPM, say 'camel', to the same repository.</div><div>3) The resulting advisory-conflict-resolution falls into the "date and version identical, pkg-list disjoint" case, which calls for an advisory merge here:</div><div>  <a href="https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/advisory.py#L222">https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/advisory.py#L222</a></div><div><br></div><div>4) advisory_merge() finds two 'collections' - one for each pkglist.packages in each advisory, neither of which is named.</div><div>5) advisory_merge() results in an advisory with two collections. One has no name, and is a package-list containing 'bear'. The other is named "None_0"[0], and is a package-list containing "camel".</div><div><br></div><div>You can see where this decision is made here:<div>   <a href="https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/advisory.py#L273-L284">https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/advisory.py#L273-L284<br></a></div><div><br></div><div>That result was surprising to me. My assumption going in was, if we were merging two advisories, we would merge their same-named collections into one collection with elements from both. But we're very carefully choosing to not-do that.</div><div><br></div><div>So, my question here is, can someone explain to me the rationale for the current behavior? I'm sure it has something to do with modularity, because half of the weird things I run into in RPM-land are because of modularity :) But I'd like to understand why we're doing what we're doing, so I can add some commentary in the test or in the merge_advisories() code itself.</div><div><br></div><div>If this turns out to be undesirable emergent behavior, then I can address it as part of this PR.</div><div><br></div><div>The test I added for all this fun is here:</div><div>  <a href="https://github.com/pulp/pulp_rpm/pull/1973/files#diff-781abba15eb7e61b9ae644ae112bf1292dfd7eb78e7162b74830d4ddb43e1868R274">https://github.com/pulp/pulp_rpm/pull/1973/files#diff-781abba15eb7e61b9ae644ae112bf1292dfd7eb78e7162b74830d4ddb43e1868R274</a></div><div><br></div><div>Thoughts very much appreciated!</div><div>G</div><div><br></div><div>[0] "None_0" is Rude. Even if this is the right behavior, we prob could stand to notice 'unnamed' collections and do something a little less painful...</div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Grant Gainey</div><div>Principal Software Engineer, Red Hat System Management Engineering</div></div></div></div></div></div></div>