Task #2273: Move OperationPostponed middleware to somewhere in pulp.app and update - Pulp

Actions

Send by e-mail Copy link

Task #2273

closed

Move OperationPostponed middleware to somewhere in pulp.app and update

Added by bmbouter over 8 years ago. Updated over 4 years ago.

Status:

CLOSED - CURRENTRELEASE

Priority:

Normal

Assignee:

dkliban@redhat.com

Category:

Sprint/Milestone:

3.0.0

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

Copy link

Updated by semyers about 8 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

Copy link

Updated by mhrivnak about 8 years ago

Sprint/Milestone set to 27

Actions

Copy link

Updated by jortel@redhat.com about 8 years ago

Sprint/Milestone changed from 27 to 28

Actions

Copy link

Updated by dkliban@redhat.com about 8 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

Copy link

Updated by bmbouter about 8 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

Copy link