Trigger a build asynchronously #61
Conversation
|
|
||
| #### HTTP Endpoints |
There was a problem hiding this comment.
I believe we should place this under a wiki page and gradually enrich it.
cmd/mistry/main.go
Outdated
| project, group it under group_name, sync the result to the | ||
| directory /tmp/yarn and use the dynamic arguments yarn.lock and foo. | ||
| Note the @ before the yarn.lock, this indicates a referral to an actual file on the | ||
| filesystem. |
There was a problem hiding this comment.
I think we can simplify this even more:
Schedule a job with a group and some parameters and put artifacts under
/tmp/yarnusing rsync. Prefixing a file name with @ will cause the contents of yarn.lock to be sent as parameters.
cmd/mistry/main.go
Outdated
|
|
||
| 2. The following sequence uses the GROUP environment variable to set the project group. | ||
| 2. Uses the GROUP environment variable to set the project group. |
There was a problem hiding this comment.
Since we're at it, I think this example isn't needed at all. A note that says that every flag can be provided as environment variable too should be sufficient.
cmd/mistry/main.go
Outdated
| 3. Schedules a build and exits early without waiting for the result by not | ||
| specifying a transport | ||
|
|
||
| $ {{.HelpName}} -host example.org --port 9090 --project yarn |
cmd/mistry/main.go
Outdated
|
|
||
| app := cli.NewApp() | ||
| app.Name = "mistry-cli" |
There was a problem hiding this comment.
Is the .Name set automatically by urfave/cli?
There was a problem hiding this comment.
cmd/mistry/main.go
Outdated
| } | ||
|
|
||
| request, err := http.NewRequest("POST", fmt.Sprintf("http://%s:%s/%s", host, port, JobsPath), bytes.NewBuffer(jrJSON)) | ||
| body, err := sendRequest(url, verbose, project, group, params) |
There was a problem hiding this comment.
Maybe better construct the JobRequest before calling this function and just pass it as a parameter? The function would then would only do what it's supposed to.
| t.Fatalf("mistry-cli stdout: %s, stderr: %s, err: %#v", cmdout, cmderr, err) | ||
| } | ||
| assertEq(cmdout, "", t) | ||
| assertEq(cmderr, "", t) |
There was a problem hiding this comment.
I think we should test here that the build has indeed run successfully.
| @@ -57,6 +57,7 @@ const ( | |||
| // Docker images/containers created by mistry. | |||
| ImgCntPrefix = "mistry-" | |||
|
|
|||
| // DateFmt the date format | |||
cmd/mistryd/job.go
Outdated
| @@ -32,9 +32,7 @@ type Job struct { | |||
| ID string | |||
|
|
|||
| // user-provided | |||
| Project string | |||
| Params types.Params | |||
| Group string | |||
There was a problem hiding this comment.
Job is a core struct and should not get everything that a JobRequest has. In the future we might add more fields and methods to JobRequest but we don't want to add them here.
On the other hand, I see why you chose to do so and perhaps we should extract those common fields into yet another type called JobInfo maybe or something like that.
Anyway, I think we should leave this for another patch.
cmd/mistryd/server.go
Outdated
| @@ -134,6 +144,12 @@ func (s *Server) HandleNewJob(w http.ResponseWriter, r *http.Request) { | |||
| } | |||
| } | |||
|
|
|||
| func (s *Server) handleNewJobAsync(j *Job, w http.ResponseWriter) { | |||
| s.Log.Printf("Scheduling %s...", j) | |||
| defer s.Work(context.Background(), j) | |||
There was a problem hiding this comment.
We don't want to leave the connection open. You could just go s.Work this.
There was a problem hiding this comment.
yes, I meant to go this
CHANGELOG.md
Outdated
| @@ -6,6 +6,7 @@ Breaking changes are prefixed with a "[BREAKING]" label. | |||
|
|
|||
| ### Added | |||
|
|
|||
| - Asynchronous build scheduling [[#61](https://github.com/skroutz/mistry/pull/61)] | |||
|
LGTM! |
Add a
mistry build --no-waitflag that triggers a build and exits without waitingPOST /jobs?async, returns an empty 201 response on successSome small random improvements
embed the JobRequest into the Job to avoid field declaration duplicationCloses #15