Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Remove docker client + queue singletons #129

Merged
merged 2 commits into from

2 participants

@vito

This should finish up the work started in inject-builder; both singletons introduced are now injected from main. Hooray!

@vito

Build is red but it looks like it just got cut off while building. It compiles and the tests pass locally for me.

@bradrydzewski

If we go this route (which I think makes sense) we should probably create stucts for all handlers and inject the database, smtp server, etc.

We don't need to tackle this now, we can refactor over time... just figured I'd bring it up to get your thoughts.

@bradrydzewski bradrydzewski merged commit 9aa5f95 into from
@vito

Agreed. I think we'll gradually push things up, and some may end up as flags/configuration/etc., and injected into the handlers.

@bradrydzewski

hey, can you take a quick look:
http://beta.drone.io/github.com/drone/drone/commit/9aa5f95e922f73ec63c45187c31de273c6c452d5

looks like you might have a change locally that needs to be pushed?

@vito

Yep, sorry; will update now.

@vito

Fixed in #130

(github flow question: will pushing to the same branch reopen a PR? can do that next time so there's less churn)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 26, 2014
  1. @vito
  2. @vito

    Runner -> BuildRunner to be more descriptive

    vito authored
    also kill some stale comments
This page is out of date. Refresh to see the latest.
View
4 cmd/drone/drone.go
@@ -123,6 +123,8 @@ func vet(path string) {
}
func run(path string) {
+ dockerClient := docker.New()
+
// parse the Drone yml file
s, err := script.ParseBuildFile(path)
if err != nil {
@@ -175,7 +177,7 @@ func run(path string) {
// loop through and create builders
for _, b := range builds { //script.Builds {
- builder := build.New(docker.DefaultClient)
+ builder := build.New(dockerClient)
builder.Build = b
builder.Repo = &code
builder.Key = key
View
11 cmd/droned/drone.go
@@ -5,7 +5,9 @@ import (
"flag"
"log"
"net/http"
+ "runtime"
"strings"
+ "time"
"code.google.com/p/go.net/websocket"
"github.com/GeertJohan/go.rice"
@@ -13,10 +15,12 @@ import (
_ "github.com/mattn/go-sqlite3"
"github.com/russross/meddler"
+ "github.com/drone/drone/pkg/build/docker"
"github.com/drone/drone/pkg/channel"
"github.com/drone/drone/pkg/database"
"github.com/drone/drone/pkg/database/migrate"
"github.com/drone/drone/pkg/handler"
+ "github.com/drone/drone/pkg/queue"
)
var (
@@ -118,6 +122,11 @@ func setupStatic() {
// setup routes for serving dynamic content.
func setupHandlers() {
+ queueRunner := queue.NewRunner(docker.New(), 300*time.Second)
+ queue := queue.Start(runtime.NumCPU(), queueRunner)
+
+ hookHandler := handler.NewHookHandler(queue)
+
m := pat.New()
m.Get("/login", handler.ErrorHandler(handler.Login))
m.Post("/login", handler.ErrorHandler(handler.Authorize))
@@ -177,7 +186,7 @@ func setupHandlers() {
m.Get("/account/admin/users", handler.AdminHandler(handler.AdminUserList))
// handlers for GitHub post-commit hooks
- m.Post("/hook/github.com", handler.ErrorHandler(handler.Hook))
+ m.Post("/hook/github.com", handler.ErrorHandler(hookHandler.Hook))
// handlers for first-time installation
m.Get("/install", handler.ErrorHandler(handler.Install))
View
2  pkg/build/docker/client.go
@@ -29,8 +29,6 @@ const (
// Enables verbose logging to the Terminal window
var Logging = true
-var DefaultClient = New() // TEMPORARY; PLEASE CONSTRUCT/INJECT YOURSELF
-
// New creates an instance of the Docker Client
func New() *Client {
c := &Client{}
View
22 pkg/handler/hooks.go
@@ -13,10 +13,19 @@ import (
"github.com/drone/go-github/github"
)
+type HookHandler struct {
+ queue *queue.Queue
+}
+
+func NewHookHandler(queue *queue.Queue) *HookHandler {
+ return &HookHandler{
+ queue: queue,
+ }
+}
+
// Processes a generic POST-RECEIVE hook and
// attempts to trigger a build.
-func Hook(w http.ResponseWriter, r *http.Request) error {
-
+func (h *HookHandler) Hook(w http.ResponseWriter, r *http.Request) error {
// handle github ping
if r.Header.Get("X-Github-Event") == "ping" {
return RenderText(w, http.StatusText(http.StatusOK), http.StatusOK)
@@ -25,7 +34,7 @@ func Hook(w http.ResponseWriter, r *http.Request) error {
// if this is a pull request route
// to a different handler
if r.Header.Get("X-Github-Event") == "pull_request" {
- PullRequestHook(w, r)
+ h.PullRequestHook(w, r)
return nil
}
@@ -160,14 +169,13 @@ func Hook(w http.ResponseWriter, r *http.Request) error {
//realtime.CommitPending(repo.UserID, repo.TeamID, repo.ID, commit.ID, repo.Private)
//realtime.BuildPending(repo.UserID, repo.TeamID, repo.ID, commit.ID, build.ID, repo.Private)
- queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) //Push(repo, commit, build, buildscript)
+ h.queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript}) //Push(repo, commit, build, buildscript)
// OK!
return RenderText(w, http.StatusText(http.StatusOK), http.StatusOK)
}
-func PullRequestHook(w http.ResponseWriter, r *http.Request) {
-
+func (h *HookHandler) PullRequestHook(w http.ResponseWriter, r *http.Request) {
// get the payload of the message
// this should contain a json representation of the
// repository and commit details
@@ -276,7 +284,7 @@ func PullRequestHook(w http.ResponseWriter, r *http.Request) {
// notify websocket that a new build is pending
// TODO we should, for consistency, just put this inside Queue.Add()
- queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript})
+ h.queue.Add(&queue.BuildTask{Repo: repo, Commit: commit, Build: build, Script: buildscript})
// OK!
RenderText(w, http.StatusText(http.StatusOK), http.StatusOK)
View
14 pkg/queue/runner.go → pkg/queue/build_runner.go
@@ -10,29 +10,29 @@ import (
"github.com/drone/drone/pkg/build/script"
)
-type Runner interface {
+type BuildRunner interface {
Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (success bool, err error)
}
-type runner struct {
+type buildRunner struct {
dockerClient *docker.Client
timeout time.Duration
}
-func newRunner(dockerClient *docker.Client, timeout time.Duration) *runner {
- return &runner{
+func NewBuildRunner(dockerClient *docker.Client, timeout time.Duration) BuildRunner {
+ return &buildRunner{
dockerClient: dockerClient,
timeout: timeout,
}
}
-func (r *runner) Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (bool, error) {
- builder := build.New(r.dockerClient)
+func (runner *buildRunner) Run(buildScript *script.Build, repo *repo.Repo, key []byte, buildOutput io.Writer) (bool, error) {
+ builder := build.New(runner.dockerClient)
builder.Build = buildScript
builder.Repo = repo
builder.Key = key
builder.Stdout = buildOutput
- builder.Timeout = r.timeout
+ builder.Timeout = runner.timeout
err := builder.Run()
View
19 pkg/queue/queue.go
@@ -1,11 +1,8 @@
package queue
import (
- "github.com/drone/drone/pkg/build/docker"
"github.com/drone/drone/pkg/build/script"
. "github.com/drone/drone/pkg/model"
- "runtime"
- "time"
)
// A Queue dispatches tasks to workers.
@@ -25,24 +22,12 @@ type BuildTask struct {
Script *script.Build
}
-var defaultQueue = Start(runtime.NumCPU(), newRunner(docker.DefaultClient, 300*time.Second)) // TEMPORARY; INJECT PLEASE
-
-var Add = defaultQueue.Add // TEMPORARY; INJECT PLEASE
-
-func Start(workers int, runner Runner) *Queue {
- // get the number of CPUs. Since builds
- // tend to be CPU-intensive we should only
- // execute 1 build per CPU.
- // must be at least 1
- // if ncpu < 1 {
- // ncpu = 1
- // }
-
+// Start N workers with the given build runner.
+func Start(workers int, runner BuildRunner) *Queue {
tasks := make(chan *BuildTask)
queue := &Queue{tasks: tasks}
- // spawn a worker for each CPU
for i := 0; i < workers; i++ {
worker := worker{
runner: runner,
View
2  pkg/queue/worker.go
@@ -17,7 +17,7 @@ import (
)
type worker struct {
- runner Runner
+ runner BuildRunner
}
// work is a function that will infinitely
Something went wrong with that request. Please try again.