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

Adds new dispatch call and deprecate the old one #1226

Merged
merged 2 commits into from Apr 6, 2021

Conversation

bmbouter
Copy link
Member

@bmbouter bmbouter commented Apr 1, 2021

This adds a new pulpcore.plugin.tasking.dispatch interface which will
replace the pulpcore.plugin.tasking.enqueue_with_reservation
interface. This also deprecates the
pulpcore.plugin.tasking.enqueue_with_reservation and causes it to
emit warnings if used.

Additionally the pulpcore.plugin.viewsets.OperationPostponedResponse
has been ported to support both the dispatch and
enqueue_with_reservation interfaces.

closes #8496

@@ -176,6 +177,21 @@ def enqueue_with_reservation(
Raises:
ValueError: When `resources` is an unsupported type.
"""
logging.warning(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be refactored to use the facilities here: #1225

@pulpbot
Copy link
Member

pulpbot commented Apr 1, 2021

Attached issue: https://pulp.plan.io/issues/8496

resp = {"task": reverse("tasks-detail", args=[result.id], request=None)}
try:
pk = task.id
except AttributeError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on here?

Copy link
Contributor

@dralley dralley Apr 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Just put an inline comment about which maps to which (RQ vs Task). PK makes it clear-ish but it would still be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new dispatch interface returns a Task, but the old enqueue_with_reservation returns a RQ.job. Both return types end up getting passed into OperationPostponedResponse (see L#21 docstring). If id is present it's the RQ.job style, otherwise it's the `Task. Maybe I should put a comment on this in the code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the comment.

This adds a new `pulpcore.plugin.tasking.dispatch` interface which will
replace the `pulpcore.plugin.tasking.enqueue_with_reservation`
interface. This also deprecates the
`pulpcore.plugin.tasking.enqueue_with_reservation` and causes it to
emit warnings if used.

Additionally the `pulpcore.plugin.viewsets.OperationPostponedResponse`
has been ported to support both the `dispatch` and
`enqueue_with_reservation` interfaces.

closes #8496
Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One typo left.

[noissue]

Co-authored-by: Matthias Dellweg <2500@gmx.de>
@mdellweg mdellweg merged commit f38f955 into pulp:master Apr 6, 2021
@bmbouter bmbouter deleted the add-new-dispatch-interface branch June 24, 2021 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants