Skip to content

Comments

PLT-6408 Framework for job server#6404

Merged
hmhealey merged 15 commits intomasterfrom
jobserver
May 18, 2017
Merged

PLT-6408 Framework for job server#6404
hmhealey merged 15 commits intomasterfrom
jobserver

Conversation

@hmhealey
Copy link
Member

Here's the framework for the job server that will be able to either run jobs (data retention, search indexing, etc) as either part of the main Mattermost server or from a standalone executable. I had to make some changes to the enterprise imports for this:

  1. Instead of being included by cmd/platform/imports.go, there's now a separate imports package so they can be included when there's a different entry point into the program.
  2. As I mentioned a couple weeks ago, we had to put the enterprise interface for jobs in the einterfaces/jobs package to avoid an import cycle of einterfaces > store > einterfaces

And a few things to note about the job server itself:

  1. The job server can be built using make build-job-server and ran with make run-job-server
  2. The code to run the job server is in the jobs package and is ran by both the platform and standalone job server located in the jobs/jobserver package.
  3. The dependencies are set up in such a way that the jobs package should only rely on the model, utils, and store packages however it could probably also use app code should we need it.
  4. The job server (standalone or otherwise) listens for config changes through a new listener system so that the utils package doesn't need to explicitly manage changes to the jobs like it does with the logger, SAML, LDAP, etc.
  5. The code around setting the jobs status and such will be implemented by the individual jobs, so it is currently unused.

Ticket Link

https://mattermost.atlassian.net/browse/PLT-6408

Checklist

@hmhealey hmhealey added 2: Dev Review Requires review by a developer Major Change The pull request is a major feature or affects large areas of the code base labels May 12, 2017
@hmhealey hmhealey added this to the v3.10.0 milestone May 12, 2017
@hmhealey hmhealey requested a review from grundleborg May 12, 2017 17:39
@grundleborg
Copy link
Contributor

I've done a cursory review of the general architecture of this, and it looks good to me. Will try and take a more detailed look at it later.

Copy link
Member

@jwilander jwilander 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, just some minor comments

api4/context.go Outdated
return c
}

l4g.Error("job_id=" + c.Params.JobId)
Copy link
Member

Choose a reason for hiding this comment

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

Debug statement you forgot to remove?

api4/context.go Outdated
return c
}

l4g.Error("job_type=" + c.Params.JobType)
Copy link
Member

Choose a reason for hiding this comment

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

Here too

api4/job.go Outdated
)

func InitJob() {
l4g.Error("Initializing post API routes")
Copy link
Member

@jwilander jwilander May 15, 2017

Choose a reason for hiding this comment

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

Shouldn't this be Info and copy/paste error saying post

@grundleborg grundleborg added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels May 17, 2017
@hmhealey hmhealey merged commit 577ed27 into master May 18, 2017
@hmhealey hmhealey deleted the jobserver branch May 18, 2017 19:06
@lindalumitchell lindalumitchell added the Tests/Not Needed New release tests not required label May 19, 2017
@esethna esethna added Docs/Not Needed Does not require documentation Changelog/Not Needed Does not require a changelog entry labels Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Major Change The pull request is a major feature or affects large areas of the code base Tests/Not Needed New release tests not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants