core-service & ml-workers integration #12
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of tracking task priority with words like “high”, “low”, etc, this commit changes task priorities to be tracked with positive integers. Lower integers are higher priority. Why? Simplicity and performance. It is much easier for a database to sort numbers than to convert words to numbers and then sort. It also simplifies the “vocabulary”, meaning the specific word used is not relevant - only the magnitude of number.
For now, I believe it is acceptable for the task-service to automatically create a task-def based off of a task creation request if a task-def of the specified name doesn’t already exist. This also helps with ensuring the core-service tests succeed in their current form, as they do not create a task-def prior to creating a task.
Including the nested data of a task-def in the data of task create request will create both the task-def and the task simultaneously. If the task-def already exists, a new one is not created and the old one is used.
This endpoint is called by ml-worker when a task is completed and the completed notebook has been uploaded to core-service. The endpoint updates the status, completed_at, locked_at, and worker_id attributes of the task.
I prefer the design pattern where a request hits an endpoint for a specific task and the server does the updating, rather than the client doing the updating and hitting a general purpose “update” endpoint where the client can update any value. I followed this pattern in 532d1f8 and continue it here by adding in an endpoint for “failing” a task. I also updated the queue tests as they used to hit the general purpose “update” endpoint - they now hit the specific complete/fail/release/etc. endpoints.
dcgoss
changed the title
Ported from core-service
Added CircleCI integration
A 204 status code delivers no data in the response. A 200 allows for data to be returned.
dhimmel
reviewed
Jul 10, 2017
| commands: | ||
| - chmod +x ecr_push.sh ecs_deploy.sh | ||
| - ./ecr_push.sh | ||
| - ./ecs_deploy.sh -t 180 -c $AWS_CLUSTER_NAME -n $AWS_SERVICE_NAME -i $AWS_ECR_ENDPOINT.dkr.ecr.us-east-1.amazonaws.com/cognoma-task-service:$CIRCLE_SHA1 |
There was a problem hiding this comment.
I recommend long args whenever you're writing shell scrips (i.e. not just typing in command line), since they're more explicit and readable. Also consider splitting long command onto multiple lines, which will help with git diff later.
| @@ -0,0 +1,544 @@ | |||
| #!/usr/bin/env bash | |||
| # source: https://github.com/silinternational/ecs-deploy | |||
There was a problem hiding this comment.
You should probably include the MIT License text in this file from https://github.com/silinternational/ecs-deploy/blob/develop/LICENSE.
| END, | ||
| run_at | ||
| LIMIT %s | ||
| ORDER BY priority, run_at |
|
Closing due to cognoma/core-service#74 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
In order to get to MVP, task-service must integrate with core-service and ml-workers.
This involves:
API changes
task_def: {attributes}in the request data will create a new task-def alongside the task. If a task-def already exists with the specified characteristics, then that task-def is used instead of creating a new one. (resolves issue Automatic task-def creation from task creation request #16)POSTto/tasks/id/complete,/tasks/id/fail,/tasks/id/releaseto update the status of a task (resolves issues Endpoint for completed task #13, Endpoint for failed task #14)Functional Tests