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

api: add logging #41

Merged
merged 2 commits into from
May 18, 2017
Merged

Conversation

diegodelemos
Copy link
Member

Signed-off-by: Diego Rodriguez diego.rodriguez@cern.ch

@diegodelemos diegodelemos force-pushed the improve-logging branch 4 times, most recently from 8016efe to 1e5324a Compare May 10, 2017 08:36
@diegodelemos diegodelemos changed the title WIP api: add logging api: add logging May 10, 2017
docs/restapi.rst Outdated
POST /jobs
----------
Create Jobs
-----------
Copy link
Member

Choose a reason for hiding this comment

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

Create jobs here and elsewhere; the second word is lowercase.


.. http:get:: /jobs/<job_id>

Returns a JSON list with all the jobs.
Returns the Job object indentified by `job_id`.
Copy link
Member

Choose a reason for hiding this comment

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

Returns the job object...

@@ -247,16 +286,52 @@ def get_job(job_id):
}

:resheader Content-Type: application/json
:statuscode 200: no error - the list has been returned.
:statuscode 200: no error - the Job has been returned.
Copy link
Member

Choose a reason for hiding this comment

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

"no error" sounds negative, rather formulate positive things positively 😄 for example "200 OK - the jo has been returned"

@tiborsimko
Copy link
Member

LGTM, other than the cosmetic commenst above. What about the merging order of #40 and #41? They currently start from the same master branch point....Will you rebase one on top of another?

@diegodelemos
Copy link
Member Author

Once #40 gets merged I will rebase on top of it and adapt the new endpoint 🙃

@@ -232,7 +271,7 @@ def get_job(job_id):

{
"job": {
"cmd": "sleep 1000",
"cmd": "echo helloworld",
Copy link
Member

Choose a reason for hiding this comment

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

true

@@ -136,7 +120,7 @@ def create_job():

{
"docker-img": "busybox",
"cmd": "sleep 1000",
"cmd": "echo helloworld",
Copy link
Member

Choose a reason for hiding this comment

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

true

Signed-off-by: Diego Rodriguez <diego.rodriguez@cern.ch>
@diegodelemos diegodelemos force-pushed the improve-logging branch 2 times, most recently from a6a9378 to 792aab5 Compare May 15, 2017 16:06
@diegodelemos diegodelemos force-pushed the improve-logging branch 3 times, most recently from d54ed28 to 0b99860 Compare May 16, 2017 12:17
* Refactors JOB_DB management towards DB switch.

Signed-off-by: Diego Rodriguez <diego.rodriguez@cern.ch>
@diegodelemos
Copy link
Member Author

Tested creating cluster from scratch. ✅ (along with reanahub/reana-workflow-engine-yadage#26)

Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

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

LGTM

@tiborsimko tiborsimko merged commit 0b99860 into reanahub:master May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants