Project

Profile

Help

Task #2273

Move OperationPostponed middleware to somewhere in pulp.app and update

Added by bmbouter about 3 years ago. Updated 6 months ago.

Status:
MODIFIED
Priority:
Normal
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 10

Description

Currently the middleware lives at https://github.com/pulp/pulp/blob/master/server/pulp/server/webservices/middleware/postponed.py

  • This needs to be moved to somewhere in pulp.app Maybe as pulp.app.middleware.postponed ?
  • Included in the new Django settings file (pulp.app.settings) so that Django uses it
  • Rethink or delete the OperationPostponed object.
  • Test that the middleware works

Question

Do we want to continue using OperationPostponedException? It is convenient to raise it, but it is strange since that is how 100% of view calls will exit for most calls.

As an alternative we could have many views return an object type that the "postponed middleware" would use to know the task id. I believe each call dispatches exactly 1 task right? Once the middleware knows the task ID I think it could just return that UUID data in a structured, REST-y way.

Comparison to 2.y

I believe in 2.y we lookup the TaskStatus info and return it as part of an operation postponed. This would cause the return data for an OperationPostponed to be the UUID and a REST-y link to the detail view for that Task. This would require the user to make an additional call to lookup that task by UUID to get its details.

Associated revisions

Revision 7aae1523 View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds OperationPostponedResponse object

This patch also removes the middleware that used to handle OperationPostponed exceptions.

closes #2273
https://pulp.plan.io/issues/2273

Revision 7aae1523 View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds OperationPostponedResponse object

This patch also removes the middleware that used to handle OperationPostponed exceptions.

closes #2273
https://pulp.plan.io/issues/2273

Revision 7aae1523 View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds OperationPostponedResponse object

This patch also removes the middleware that used to handle OperationPostponed exceptions.

closes #2273
https://pulp.plan.io/issues/2273

Revision f84ca21a View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds docs for OperationPostponedResponse object

re #2273
https://pulp.plan.io/issues/2273

Revision f84ca21a View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds docs for OperationPostponedResponse object

re #2273
https://pulp.plan.io/issues/2273

Revision f84ca21a View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Adds docs for OperationPostponedResponse object

re #2273
https://pulp.plan.io/issues/2273

Revision 263e156d View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Fixes path for UserFacingTask in the viewsets docs

re #2273
https://pulp.plan.io/issues/2273

Revision 263e156d View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Fixes path for UserFacingTask in the viewsets docs

re #2273
https://pulp.plan.io/issues/2273

Revision 263e156d View on GitHub
Added by dkliban@redhat.com almost 3 years ago

Fixes path for UserFacingTask in the viewsets docs

re #2273
https://pulp.plan.io/issues/2273

History

#1 Updated by semyers about 3 years ago

  • Groomed changed from No to Yes
  • Sprint Candidate changed from No to Yes

bmbouter wrote:

Currently the middleware lives at https://github.com/pulp/pulp/blob/master/server/pulp/server/webservices/middleware/postponed.py

  • This needs to be moved to somewhere in pulp.app Maybe as pulp.app.middleware.postponed ?
  • Included in the new Django settings file (pulp.app.settings) so that Django uses it
  • Rethink or delete the OperationPostponed object.
  • Test that the middleware works

All of these are good.

Question

Do we want to continue using OperationPostponedException? It is convenient to raise it, but it is strange since that is how 100% of view calls will exit for most calls.

Being of the school that exceptions should generally only be raised in exceptional circumstances, I think no.

As an alternative we could have many views return an object type that the "postponed middleware" would use to know the task id. I believe each call dispatches exactly 1 task right? Once the middleware knows the task ID I think it could just return that UUID data in a structured, REST-y way.

Views return responses, so I like the idea of an "OperationPostponedResponse" that contains enough information to be correctly serialized and returned to the user. In discussing it with the team, the least amount of information needed to return in this response is a list of URLs (one or more) to postponed tasks spawned during this request/response cycle. We could easily return only the UUIDs or Task(/TaskResult?) model instances and use something (such as a middleware) to ensure that the Response object is correctly serialized based on the incoming Request.

Comparison to 2.y

I believe in 2.y we lookup the TaskStatus info and return it as part of an operation postponed. This would cause the return data for an OperationPostponed to be the UUID and a REST-y link to the detail view for that Task. This would require the user to make an additional call to lookup that task by UUID to get its details.

Responding with any more information than links to the results seems redundant, since this response is saying "I'm not done yet, check back later...", so I'm not even sure any TaskStatus information is useful; probably just the links is enough.

#2 Updated by mhrivnak about 3 years ago

  • Sprint/Milestone set to 27

#3 Updated by jortel@redhat.com almost 3 years ago

  • Sprint/Milestone changed from 27 to 28

#4 Updated by dkliban@redhat.com almost 3 years ago

I like the idea of creating an OperationPostponedResponse object that inherits from HttpResponse0. The OperationPostponedResponse would take a required keyword argument 'spawned_task_ids'. The view code would then call apply_async*() for a task and get a task id. This could be done for multiple tasks. The list of all the task ids would then be used to instantiate the OperationPostponedResponse object.

The `__init__` for OperationPostponedResponse would set the content of the response to something like this:

{
"task_group_id": null
 "tasks": [{"_href": "/pulp/api/v3/tasks/7744e2df-39b9-46f0-bb10-feffa2f7014b/",
                    "task_id": "7744e2df-39b9-46f0-bb10-feffa2f7014b" }]
}

[0] https://docs.djangoproject.com/en/1.9/ref/request-response/#django.http.HttpResponse

#5 Updated by bmbouter almost 3 years ago

@dkliban, +1 to the approach you outline. My only suggestion would be consider initializing the init of the OperationPostponedResponse with the AsyncResult objects directly. This would allow us to avoid unpacking the my_async_result.task_id over and over again while initializing OperationPostponedResponse objects all over the view layer.

#6 Updated by dkliban@redhat.com almost 3 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to dkliban@redhat.com

#7 Updated by dkliban@redhat.com almost 3 years ago

  • Status changed from ASSIGNED to POST

#8 Updated by dkliban@redhat.com almost 3 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100

#9 Updated by semyers almost 3 years ago

  • Tags Pulp 3 added

#10 Updated by bmbouter over 1 year ago

  • Sprint set to Sprint 10

#11 Updated by bmbouter over 1 year ago

  • Sprint/Milestone deleted (28)

#12 Updated by daviddavis 6 months ago

  • Sprint/Milestone set to 3.0

#13 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF