-
Notifications
You must be signed in to change notification settings - Fork 79
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
Retrieve functionality breaks semver, requires task monitoring #1038
Comments
Ok, I need to split my initial response into three parts: First, regarding semver: Rightly or wrongly, I have never considered pulpcore's semver promise to apply to pulp_deb. We even documented our own pulp_deb version semantics and those are not equivalent to semver. That being said, looking at the location and the pulpcore docs now, I don't think this is a communication that anyone is likely to see. As a result, I have started a forum discussion. Either pulp_deb needs to start adhearing to semver, or the documentation needs to become much clearer that not all plugins are doing so. Second, regarding the fact that the API now returns a task: This was always a complete oversight/accident/bug that these API endpoints did not return a task. Especially considering that when supplying the Third, regarding the retrieve logic: Actually I am not sure from your post if the retrieve logic is problematic for you separately from the task thing? I presume for now, you have simply gone back to the 3.1.* version? We are currently quite busy on our end with not pulp_deb stuff, so there are unlikely to be any (non-bugfix) releases for a while. Hopefully that gives us some time to chart a constructive path forwards. |
Thanks for the response. I was under the impression that pulp_deb used semver so that's good to know that you don't and have documented how you version pulp_deb. I also appreciate the forum discussion around plugin version specifications. I guess we can wait until that discussion is resolved before we talk more about pulp_deb's version specification. Even if you ultimately decide not to use semver, maybe we can contribute feedback because I think the pulp_dev version spec could be more friendly for users. I totally agree that adding content to a repo requires returning a task and not using a task was an oversight. For users like us though that aren't using the repository field, I wonder what reason there is to return a task? In other words, if we're just creating a For the package creation API endpoint, we just call it, return the resulting Pulp task to the user, and they can query its status. The same thing for adding a package to a repo: we look up the package id and PackageReleaseComponent id and then call the repository modify endpoint with both and return the resulting modify task to the user. Creating releases is much more complicated and we've tried to simplify it by having a single endpoint creates a release and adds it to a repo. This creates the release, the release components, and the release architectures, and then adds all of these objects to a repo. We then return the repository modify task to our users and we can do this because only the last step launches a task. Getting back tasks for each object would mean our users would have to create the individual pieces (components, architectures, releases) and then add all these pieces to a repo. We'd like to avoid that if possible. The retrieve logic is not a problem for us aside from the API change where these endpoints now return tasks. I agree with you that rolling forward makes sense. I've mentioned a couple of options and I think there's maybe even a third option: given that the release endpoint needs to return a task, perhaps this change could also add fields
But I am guessing this would be a major piece of work and probably not as simple as some of the other options I mentioned. For right now, we're on 3.2 but we have the commit in question reverted as a patch. This isn't ideal though as it makes upgrading harder and we just went through the trouble of getting rid of our local fork once the package source work was merged. |
For the third option I mentioned, I have a small proof of concept here that would allow users to create releases with architectures and components: |
I think that this is certainly technically possible, and the only reasons I can come up with not to do it, are long the lines of: "No other plugins has API endpoints that sometimes return objects and sometimes tasks. This is perhaps a little counter intuitive." But then again, other plugins don't have "structure content" so maybe there is no reason to be different here. Regarding your proposal for new parameters on the I am imagining a future release where all the That way the In conclusion: I think I am starting to see things your way. That being said, bandwidth being what it is, I don't know when we might work on this, but I can promise to revisit by the next Y release (whenever that will be). Actually I believe the next Y release may be a "declare compatibility against a new breaking change pulpcore version release", in which case I would revisit for the next (proper) Y release after that. 😉 |
I'm not so confident this is easily possible. For proper operation with the existing client tooling, the apispec must accurately describe the api response. And I'm not sure whether drf-spectacular or openapi-generator can deal with polymorphism nicely. Sure it would be interesting to figure this out. OTOH, and this is much more philosophical, having designed pulp to be about content AND repositories instead of content IN repositories was IMHO a bad decision. And it really didn't work out when we added RBAC support. My take is: Content creation should always happen inside a repository. And not being in a repository, content is subject to be cleaned up without further warning (Yes, we have ORPHAN_PROTECTION_TIME and "touch" but those are simply band-aids for the same design flaw.). |
The fact that That said, if we could specify |
A recent change was introduced that made a backward incompatible change to the API. I would think this violates semver as it says you must "increment MAJOR version when you make incompatible API changes". The only exception it calls out are bug fixes.
That aside, the change is problematic for us. In our system, users make a single web request to add a new release to a repo which in turn finds or creates a Release along with the necessary ReleaseArchitectures and ReleaseComponents, and then adds all the necessary objects to the repo. We don't have a background job system nor a way to monitor Pulp tasks. So getting back tasks to monitor is problematic for us.
I wonder if it would be possible to solve both these problems by either:
repository
is set and otherwise return the new architecture/component.The text was updated successfully, but these errors were encountered: