Skip to content

Refactor project documentation syncing tasks #3143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

agjohnson
Copy link
Contributor

This updates the syncing tasks to execute in a guaranteed fashion. Before this
change, it was possible for broadcast tasks and symlink deletion tasks to
execute concurrently. This changes all the broadcast task calls into function
calls from a singular broadcast task.

In more detail:

  • The finish_build task was executed as a task from the webs, and kicked off
    broadcast tasks to update files. Instead, Version update operations were move
    to the build process, through the API. The webs only the one broadcast task
  • Version API was given the same APIRestrictedPermission as other end points, it
    can now be updated by staff

This updates the syncing tasks to execute in a guaranteed fashion. Before this
change, it was possible for broadcast tasks and symlink deletion tasks to
execute concurrently. This changes all the broadcast task calls into function
calls from a singular broadcast task.

In more detail:

* The `finish_build` task was executed as a task from the webs, and kicked off
  broadcast tasks to update files. Instead, Version update operations were move
  to the build process, through the API. The webs only the one broadcast task
* Version API was given the same APIRestrictedPermission as other end points, it
  can now be updated by staff
@agjohnson agjohnson added the PR: work in progress Pull request is not ready for full review label Oct 4, 2017
@agjohnson agjohnson requested a review from ericholscher October 4, 2017 20:14
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks good. Any reason not to do the fileify and udpate_search in process too?

@ericholscher
Copy link
Member

I guess because we only want them to happen once?

@agjohnson
Copy link
Contributor Author

Yeah, i suppose we only want them to happen once. There is certainly a better pattern here, using Celery chains[1] or something eventually. I actually wonder if this causing problems on webs already, where downloads and search are updated concurrently to the file syncing tasks. Seems it would be.

I can polish up my celery test branch, there wasn't much to the upgrade, and it buys us some flexibility in task execution.

1: http://docs.celeryproject.org/en/latest/userguide/canvas.html#chains

@agjohnson agjohnson merged commit 60d2609 into master Oct 5, 2017
@agjohnson agjohnson deleted the refactor-project-tasks branch October 5, 2017 02:23
@agjohnson agjohnson removed the PR: work in progress Pull request is not ready for full review label Oct 5, 2017
ericholscher pushed a commit that referenced this pull request Oct 9, 2017
This updates the syncing tasks to execute in a guaranteed fashion. Before this
change, it was possible for broadcast tasks and symlink deletion tasks to
execute concurrently. This changes all the broadcast task calls into function
calls from a singular broadcast task.

In more detail:

* The `finish_build` task was executed as a task from the webs, and kicked off
  broadcast tasks to update files. Instead, Version update operations were move
  to the build process, through the API. The webs only the one broadcast task
* Version API was given the same APIRestrictedPermission as other end points, it
  can now be updated by staff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants