Project

Profile

Help

Task #2399

Convert celery task repo.sync to Pulp 3

Added by ttereshc almost 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 19

Description

Task name in Pulp 2: `pulp.server.managers.repo.sync.sync`.
Task implementation in Pulp 2: https://github.com/pulp/pulp/blob/3.0-dev/server/pulp/server/controllers/repository.py#L694

The 2.y implementation does a huge amount of things we don't need to do anymore. The 3.0 implementation should do the following:

1. Query for the instance of the Importer's Master model that needs to be synced
2. Cast that Importer master instance to a detail instance
3. Set the working_dir attribute on the detail instance. This was part of issue #2456
4. Call sync (it should take no arguments)


Checklist


Related issues

Related to Pulp - Task #2380: Create a redmine task for each 2.y celery task to be converted to 3.0 CLOSED - CURRENTRELEASE Actions
Related to Pulp - Task #2456: Add working directory management to sync/publish task plumbing. CLOSED - NOTABUG Actions

Associated revisions

Revision 8401606b View on GitHub
Added by amacdona@redhat.com over 2 years ago

Port sync task for Pulp 3.0

closes #2399

Revision 8401606b View on GitHub
Added by amacdona@redhat.com over 2 years ago

Port sync task for Pulp 3.0

closes #2399

Revision 8401606b View on GitHub
Added by amacdona@redhat.com over 2 years ago

Port sync task for Pulp 3.0

closes #2399

History

#1 Updated by ttereshc almost 3 years ago

  • Related to Task #2380: Create a redmine task for each 2.y celery task to be converted to 3.0 added

#2 Updated by ttereshc almost 3 years ago

  • Tags Pulp 3 added

#3 Updated by bmbouter almost 3 years ago

Here are questions I have:

1) What are the arguments for the task? scheduled_calls is not part of 3.0 so the signature would become: def sync(repo_id, sync_config_override=None) or maybe def sync(importer_id, importer_config_override=None)

2) Will validation of the importer's config occur in the task AND in the view layer? The 2.y code does it in both.

3) How will we handle unit counts? Is that going to be implicit in the unit creation or will the task have to "fully rebuild" the unit counts at the end?

#4 Updated by mhrivnak almost 3 years ago

bmbouter wrote:

Here are questions I have:

1) What are the arguments for the task? scheduled_calls is not part of 3.0 so the signature would become: def sync(repo_id, sync_config_override=None) or maybe def sync(importer_id, importer_config_override=None)

Having it take a reference to the importer seems most appropriate. The importer is the thing doing the sync, and the thing that has the data necessary to do the sync.

2) Will validation of the importer's config occur in the task AND in the view layer? The 2.y code does it in both.

Seems reasonable. I'm not sure the task should make any assumptions about how it got queued, or how good the validation was at that point. Generally in multi-service design, the less trust that's required between services, the more robust the system will be. "Every service for itself."

3) How will we handle unit counts? Is that going to be implicit in the unit creation or will the task have to "fully rebuild" the unit counts at the end?

Do you mean managing the repo counts that are stored on the repo? I think we're going to try auto-generating that on the fly in pulp 3 when a repo endpoint handles a request, which is much easier and cheaper to do with a relational DB than mongodb.

#5 Updated by ttereshc almost 3 years ago

mhrivnak wrote:

bmbouter wrote:

Here are questions I have:

1) What are the arguments for the task? scheduled_calls is not part of 3.0 so the signature would become: def sync(repo_id, sync_config_override=None) or maybe def sync(importer_id, importer_config_override=None)

Having it take a reference to the importer seems most appropriate. The importer is the thing doing the sync, and the thing that has the data necessary to do the sync.

What do you mean by importer_id? UUID? repo + importer name?
Is it possible that importer will be deleted and then created again after the task was created? Or is deletion of importer going to be a celery task as well (and not just something performed in a view) so it is safe to refer to importer directly?

#6 Updated by mhrivnak almost 3 years ago

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

Either way, it is possible that the importer will have been removed and recreated between the time this task is queued and the time it runs.

My inclination is to reference the natural key. Users have an opportunity to give importers a meaningful identifier. If they replace an importer with one that has the same identifier, it's reasonable to assume that the new importer was intended to take the place of the original.

#7 Updated by mhrivnak almost 3 years ago

  • Groomed changed from Yes to No

#8 Updated by mhrivnak almost 3 years ago

  • Groomed changed from No to Yes

I feel confident that the remaining questions are implementation details, and can be hammered out while working on the task.

#9 Updated by mhrivnak almost 3 years ago

  • Sprint/Milestone set to 30

#10 Updated by bmbouter almost 3 years ago

  • Description updated (diff)

Updating the description based on discussion during sprint planning.

#11 Updated by bmbouter almost 3 years ago

  • Related to Task #2456: Add working directory management to sync/publish task plumbing. added

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

  • Description updated (diff)

#13 Updated by bmbouter almost 3 years ago

  • Checklist item add documentation to the pulp.plugin.Importer docstring about the working_dir added

#14 Updated by mhrivnak over 2 years ago

  • Sprint/Milestone changed from 30 to 36

#15 Updated by amacdona@redhat.com over 2 years ago

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

#16 Updated by bmbouter over 2 years ago

  • Description updated (diff)

This story includes creating and deleting the working directory but it should not. The creation of the working directory is done by call and deleted by on_success() or on_failure().

#17 Updated by amacdona@redhat.com over 2 years ago

  • Description updated (diff)

I removed many of the steps for this story. Since importer/publisher overrides are no longer a part of the MVP, validation is also no longer necessary since that will take place before the configuration is saved to the db.

#18 Updated by bmbouter over 2 years ago

I think some of the removed deliverables need to come back.

  • validating the importer
  • setting the current working directory for the interpreter and then unsetting it at the end

#19 Updated by bmbouter over 2 years ago

Per discussion on pulp-dev0 I'll suggest that these are the two pieces that we need to add back:

  • raise a ValueError('feed cannot be null') if the feed is null <--- this is business logic
  • setting the current working directory for the interpreter and then unsetting it at the end

[0]: https://www.redhat.com/archives/pulp-dev/2017-April/msg00007.html

#20 Updated by mhrivnak over 2 years ago

  • Sprint/Milestone changed from 36 to 37

#21 Updated by jortel@redhat.com over 2 years ago

  • Sprint/Milestone changed from 37 to 38

#22 Updated by amacdona@redhat.com over 2 years ago

  • Status changed from ASSIGNED to POST

#23 Updated by amacdona@redhat.com over 2 years ago

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

#24 Updated by bmbouter over 1 year ago

  • Sprint set to Sprint 19

#25 Updated by bmbouter over 1 year ago

  • Sprint/Milestone deleted (38)

#26 Updated by daviddavis 6 months ago

  • Sprint/Milestone set to 3.0

#27 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF