Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

1120671 - missing operation from reaper and monthly tasks #1216

Merged
merged 5 commits into from Oct 13, 2014
Merged

1120671 - missing operation from reaper and monthly tasks #1216

merged 5 commits into from Oct 13, 2014

Conversation

dkliban
Copy link
Member

@dkliban dkliban commented Oct 8, 2014

I also added documentation about these tasks

I also added documentation about these tasks
@dkliban
Copy link
Member Author

dkliban commented Oct 8, 2014

@dkliban dkliban changed the title 1120671 - missing operation from reaper and monthly tasks [work in progress] 1120671 - missing operation from reaper and monthly tasks Oct 8, 2014
result = super(Scheduler, self).apply_async(entry, publisher, **kwargs)
# Create a new task status with the task id and tags.
task_status = TaskStatus(task_id=result.id, task_type=entry.name,
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm reading this correctly, I think this will create a TaskStatus record for every task that the scheduler dispatches. That is definitely not what we want.

What might be a better solution is to follow a model like this: https://github.com/dkliban/pulp/blob/1120671/server/pulp/server/tasks/repository.py#L172

In that case, we can't derive the appropriate tags in advance. So we have the pulp.server.tasks.repository.publish task that gets queued. When it runs, it generates the tags and queues the actual publish operation, which is pulp.server.managers.repo.publish.publish.

So for this case, we could create a new intermediate task whose only job is to add these special pulp-specific tasks.

Also, I think it's likely still wrong that the publish and sync_with_auto_publish tasks here have base=Task: https://github.com/dkliban/pulp/blob/1120671/server/pulp/server/tasks/repository.py#L171

That may or may not explain some of the task list entries that are missing information.

Let's talk through this in the morning and re-evaluate the approach.

Also added a couple of unit tests to make sure that the schedling
task actually calls the apply_async for the task that does the work
@dkliban
Copy link
Member Author

dkliban commented Oct 9, 2014

This PR is also relevant to https://bugzilla.redhat.com/show_bug.cgi?id=1101152

@dkliban dkliban changed the title [work in progress] 1120671 - missing operation from reaper and monthly tasks 1120671 - missing operation from reaper and monthly tasks Oct 9, 2014
self.assertTrue(reaper_apply_async.called)

@mock.patch('pulp.server.maintenance.monthly.monthly_maintenance.apply_async')
def test_reap_expired_documents_apply_async(self, monthly_apply_async):
Copy link
Contributor

Choose a reason for hiding this comment

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

this function name is the same as the one above

@bmbouter bmbouter self-assigned this Oct 13, 2014
task that has been executed on the pulp server. The length of history depends on the settings
described below.

In addition to tasks launched using pulp-admin, 'reaper' and 'monthly' tasks appear in the list.
Copy link
Member

Choose a reason for hiding this comment

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

s/pulp-admin/pulp-admin or the API/

Copy link
Member

Choose a reason for hiding this comment

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

Consider the use of double backticks (``) around terms throughout this paragraph where a single quote is used. Backticks have a certain restructured text meaning that would be better to identify variable names and such. Build the docs manually or look on RTD for examples of how the existing text looks.

@bmbouter
Copy link
Member

Fix the formatting of docs, check on the _args, *_kwargs arguments to the two tasks, and move the repository.py changes to its own PR, and I think it's good to merge.

dkliban added a commit that referenced this pull request Oct 13, 2014
1120671 - missing operation from reaper and monthly tasks
@dkliban dkliban merged commit 8491c0e into pulp:2.5-dev Oct 13, 2014
@dkliban dkliban deleted the 1120671 branch October 13, 2014 18:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants