Skip to content
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

refactor(tasks): separate coordinator and middleware #14779

Merged
merged 1 commit into from
Aug 23, 2019

Conversation

GeorgeMac
Copy link
Contributor

@GeorgeMac GeorgeMac commented Aug 23, 2019

This is a WIP

This separates the current task coordinators reponsibilities into both a coordinator type and a task service middleware.

The task service middleware handles delegation to a persistent task service implementation and then notifies a middlware.Coordinator implementation that the action has taken place.
It is the job of the new coordinator.Coordinator to delegate the appropriate actions to a backend.Scheduler implementation.

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Well-formatted commit messages
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Documentation updated or issue created (provide link to issue/pr)
  • Signed CLA (if not already signed)

Copy link
Contributor

@lyondhill lyondhill left a comment

Choose a reason for hiding this comment

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

LGTM :) mostly just comments, with one little feature request. Approving :)

task/backend/coordinator.go Outdated Show resolved Hide resolved
task/backend/coordinator.go Show resolved Hide resolved
task/backend/coordinator.go Outdated Show resolved Hide resolved
@GeorgeMac GeorgeMac force-pushed the gm/refactor-coordinator branch 2 times, most recently from cf1c12a to 4567c7f Compare August 23, 2019 18:40
@GeorgeMac GeorgeMac merged commit 0cc9caa into master Aug 23, 2019
@GeorgeMac GeorgeMac deleted the gm/refactor-coordinator branch August 23, 2019 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants