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

feat(sdk, hatchery, worker): get queue from server side events #3376

Merged
merged 10 commits into from
Oct 8, 2018

Conversation

fsamin
Copy link
Member

@fsamin fsamin commented Sep 28, 2018

  1. Description
  2. Related issues
  3. About tests
  4. Mentions

@ovh/cds

}

//Client is the default client used for requests.
var sseClient = &http.Client{}

Choose a reason for hiding this comment

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

sseClient is unused

@fsamin

This comment has been minimized.

}

// RequestSSEGet takes the uri of an SSE stream and channel, and will send an Event
// down the channel when recieved, until the stream is closed. It will then
Copy link
Member

Choose a reason for hiding this comment

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

recieved => received

chanSSEvt := make(chan SSEvent)
sdk.GoRoutine("RequestSSEGet", func() {
for ctx.Err() == nil {
time.Sleep(5 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

Sleep should be at the end of the loop, if ctx.Err() != nil in that delay a useless SSE request will be prepared.

}

if len(bs) < 2 {
continue
Copy link
Member

Choose a reason for hiding this comment

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

In the case where err == io.EOF, we should break.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

currEvent.Data = bytes.NewBuffer(bytes.TrimSpace(spl[1]))
evCh <- *currEvent
}
if err == io.EOF {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be just after if err != nil && err != io.EOF

Copy link
Member Author

Choose a reason for hiding this comment

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

done

if errParse != nil {
errs <- sdk.WrapError(errParse, "Unable to load jobs, failed to parse API Time")
continue
if _, err := c.GetJSON("/queue/workflows", &queue); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

missing query params modelType & ratio:

url, _ := url.Parse("/queue/workflows")
q := url.Query()	
if ratioService != nil {	
    q.Add("ratioService", fmt.Sprintf("%d", *ratioService))	
}	
if modelType != "" {	
    q.Add("modelType", modelType)	
}	
url.RawQuery = q.Encode()
_, header, _, errReq := c.RequestJSON(http.MethodGet, url.String(), nil, &queue, SetHeader(RequestedIfModifiedSinceHeader, t0.Format(time.RFC1123)))

On master branch, the oldJobsTicker does not do that, but it's a mistake. I don't fix it on master to avoid conflict with this branch. ie: hatchery swarm with ratioService 100% receives 'old' jobs without service, but it's not expected. The ticket jobsTicker is ok (no worker without service send to hatchery swarm)

// wait for the grace time before pushing the job in the channel
go func() {
time.Sleep(time.Duration(graceTime) * time.Second)
job, err := c.QueueJobInfo(int64(jobRunID))
Copy link
Member

Choose a reason for hiding this comment

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

Job received by SSE should contains modelType and ContainsService attributes.
So, an hatchery openstack avoid call QueueJobInfo for each job type docker,
a hatchery marathon avoid cal QueueJobInfo for each job containsService, etc...

This will be the same behaviour with existing call /queue/workflow, with queryParams modelType and ratioService

Copy link
Member Author

Choose a reason for hiding this comment

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

@bnjjj bnjjj merged commit ec92876 into master Oct 8, 2018
@fsamin fsamin deleted the feat-queue-polling-from-sse branch October 10, 2018 17:44
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.

6 participants