Project

Profile

Help

Task #2273

closed

Move OperationPostponed middleware to somewhere in pulp.app and update

Added by bmbouter over 7 years ago. Updated over 3 years ago.

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

100%

Estimated time:
Platform Release:
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
Sprint:
Sprint 10
Quarter:

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.

Actions #1

Updated by semyers over 7 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.

Actions #2

Updated by mhrivnak over 7 years ago

  • Sprint/Milestone set to 27
Actions #3

Updated by jortel@redhat.com over 7 years ago

  • Sprint/Milestone changed from 27 to 28
Actions #4

Updated by dkliban@redhat.com over 7 years ago

I like the idea of creating an OperationPostponedResponse object that inherits from HttpResponse[0]. 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

Actions #5

Updated by bmbouter over 7 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.

Actions #6

Updated by dkliban@redhat.com over 7 years ago

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

Added by dkliban@redhat.com over 7 years ago

Revision 7aae1523 | View on GitHub

Adds OperationPostponedResponse object

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

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

Added by dkliban@redhat.com over 7 years ago

Revision 7aae1523 | View on GitHub

Adds OperationPostponedResponse object

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

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

Actions #7

Updated by dkliban@redhat.com over 7 years ago

  • Status changed from ASSIGNED to POST
Actions #8

Updated by dkliban@redhat.com over 7 years ago

  • Status changed from POST to MODIFIED
  • % Done changed from 0 to 100
Actions #9

Updated by semyers over 7 years ago

  • Tags Pulp 3 added

Added by dkliban@redhat.com over 7 years ago

Revision f84ca21a | View on GitHub

Adds docs for OperationPostponedResponse object

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

Added by dkliban@redhat.com over 7 years ago

Revision f84ca21a | View on GitHub

Adds docs for OperationPostponedResponse object

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

Added by dkliban@redhat.com over 7 years ago

Revision 263e156d | View on GitHub

Fixes path for UserFacingTask in the viewsets docs

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

Added by dkliban@redhat.com over 7 years ago

Revision 263e156d | View on GitHub

Fixes path for UserFacingTask in the viewsets docs

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

Actions #10

Updated by bmbouter about 6 years ago

  • Sprint set to Sprint 10
Actions #11

Updated by bmbouter about 6 years ago

  • Sprint/Milestone deleted (28)
Actions #12

Updated by daviddavis almost 5 years ago

  • Sprint/Milestone set to 3.0.0
Actions #13

Updated by bmbouter almost 5 years ago

  • Tags deleted (Pulp 3)
Actions #14

Updated by bmbouter over 4 years ago

  • Status changed from MODIFIED to CLOSED - CURRENTRELEASE

Also available in: Atom PDF