Allow users to specify distribution and component when modifying content

[daviddavis]

Allow repositories/deb/apt/{pulp_id}/modify/ requests to add/remove packages using optional distribution and component parameters. The task will create or remove matching release structure content while preserving package-only behavior when both parameters are omitted.

fixes #1491

[quba42]

First of all: I really appreciate this change! I started by testing some workflows using this, against my own assumptions and expectations. Overall the current state makes very reasonable design choices. It appears to work well (though I have not yet tested the content removal workflows 😉)

That being said, on the details, there are several design choices I would like to have some (open ended) discussion on. To avoid this turning to chaotic, I am going to avoid GitHub's review feature for now, and will instead open a thread for each independent thing I would like to discuss. I may need a bit of time to add each thread below.

[quba42]

I am going to leave it at that for now. Note that while I may have produced a lot of text, all of the above are posed as open questions. Disagreeing with any implied changes is perfectly fine. If, after some discussion, we conclude we want to stay with the current design that is a perfectly reasonable outcome.

[daviddavis]

@quba42 I think I've addressed everything. Please let me know what you think.

[balasankarc]

cc @stanhu FYI as you worked on pulp/pulp-cli-deb#260

[daviddavis]

@balasankarc @stanhu reading pulp/pulp-cli-deb#245, it seems like maybe the field here should be distributions instead of distribution? So that way you can add a package to multiple distributions when you add it to the repo. I don't think that would be too difficult to do.

[quba42]

I found two more minor changes I would like to see.

With those final changes I am happy to merge. I have manually tested many workflows I wanted to see (like adding a package already in the repo to another ReleaseComponent, removing packages from just one ReleaseComponent. Removing a package from the repo and all ReleaseComponents (by not providing the new params). Adding multiple packages. Re-running a command that results in a noop because everything is already present, etc.

I also looked over the new automatic tests which provide good coverage for those same workflows I most wanted to test manually. 👍

[quba42]

One thing I did not test is what happens when you provide both add_content_units and remove_content units and a component or distribution in a single call. From the way the individual calls work and how the code is structured I am pretty confident it will simply be the conjunction of the two individual calls. I am not sure how important this corner case is. I would expect the vast majority of users to split that request up into two calls, just because the request "add these packages to this ReleaseComponent and also remove these other packages from the same ReleaseComponent at the same time" is conceptually pretty weird. 😄

If you end up requesting the same package be added and also removed in the same call, I don't particularly care what happens since such a clearly contradictory request arguably should result in undefined behavior.

[daviddavis]

I agree that most users will probably add/remove in a single call. It should handle requests to both add and remove content in a single call though by both adding and removing the packages that the user requests to add and remove.

The one edge case you alluded to where there is an overlap of content is not obvious for users though without looking at the code (I believe that the answer is that the content gets added). I agree though that we shouldn't worry about it since it's obviously unusual and contradictory.

[daviddavis]

I went ahead and added a new functional test (test_add_and_remove_packages_in_same_request) to test adding/removing packages a single request.