api: openapi specification generation #40

diegodelemos · 2017-05-05T16:30:35Z

Adds CLI to generate OpenAPI spec out of docstrings.
Adds initial test to validate spec.

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

tiborsimko · 2017-05-12T07:36:28Z

reana_job_controller/cli.py

+
+import json
+
+import click


Please add click to the required dependencies in setup.py. (And test in a fresh environment for others.)

click comes with Flask 0.11:

Collecting click>=2.0 (from Flask>=0.11->reana-job-controller==0.0.1.dev20170123)

As the saying goes, "better be safe than sorry" :), so an explicit click dependency may be safer than an implicit one, in the case when Flask would decide to make it optional. (Well, that's not probable...)

tiborsimko · 2017-05-12T07:38:08Z

run-tests.sh

@@ -23,7 +23,7 @@
 pydocstyle reana_job_controller && \
 isort -rc -c -df **/*.py && \
 check-manifest --ignore ".travis-*" && \
-sphinx-build -qnNW docs docs/_build/html && \
+sphinx-build -qnN docs docs/_build/html && \


It might be good to generate a "temporary" OpenAPI specs and compare it with previously-generated and committed one, so that developers are alerted if they changed something important and need to regenerate it.

tiborsimko · 2017-05-12T07:39:58Z

reana_job_controller/app.py

+def get_openapi_spec():
+    """Get OpenAPI Spec.
+
+    FIXME add openapi spec


Perhaps we should make issues for these FIXMEs if we don't have time to attack them within a day or two?

Absolutely, there are things such as validating OpenAPI spec file which is generated in run_tests.sh which is far from being acceptable. I will open issues 👍

tiborsimko · 2017-05-12T07:42:05Z

reana_job_controller/cli.py

+@openapi.command()
+@click.argument('output', type=click.File('w'))
+@with_appcontext
+def create(output):


It would be good to plug CLI call into Sphinx documentation. Perhaps a new "CLI" chapter, perhaps simply under "Getting started" for now.

diegodelemos · 2017-05-15T07:28:23Z

@tiborsimko

tiborsimko · 2017-05-15T08:06:25Z

docs/gettingstarted.rst

+
+.. code-block:: console
+
+   $ flask openapi create output_file


FLASK_APP=reana_job_controller/app.py flask openapi create openapi.json

tiborsimko · 2017-05-15T08:06:45Z

docs/gettingstarted.rst

+CLI
+---
+
+The REANA Job Controller package offers the possibility to create its OpenAPI specification file using the flask CLI.


using the Flask CLI:

tiborsimko · 2017-05-15T08:10:17Z

MANIFEST.in

@@ -23,6 +23,7 @@ include COPYING
 include *.rst
 include *.sh
 include pytest.ini
+include docs/openapi.json


We need to include this file in the repository, otherwise RTFD wouldn't build the documentation.

tiborsimko · 2017-05-15T08:11:00Z

run-tests.sh

 python setup.py test && \
-sphinx-build -qnNW -b doctest docs docs/_build/doctest && \
+sphinx-build -qnN -b doctest docs docs/_build/doctest && \
+rm -rf docs/openapi.json && \


See my comment above regarding having the OpenAPI spec file in the package.

tiborsimko · 2017-05-15T08:30:23Z

reana_job_controller/app.py

+
+    ---
+    get:
+      description: Get a Job by its id


This should be a bit more descriptive, for example:

"Returns details about a given job."

tiborsimko · 2017-05-15T08:40:57Z

reana_job_controller/app.py

+         type: string
+      responses:
+        200:
+          description: The Job.


Ditto for description of responses:

200 Request succeeded. The response contains details about the given job ID.

404 Request failed. The given job ID does not seem to exist.

tiborsimko · 2017-05-15T08:42:42Z

reana_job_controller/app.py

-                  "status": "started"
+    ---
+    get:
+      description: Get all Jobs


Returns list of all active jobs.

tiborsimko · 2017-05-15T08:44:07Z

reana_job_controller/app.py

+       - application/json
+      responses:
+        200:
+          description: Job list.


200: Request succeeded. The response contains the list of all active jobs.

tiborsimko · 2017-05-15T08:45:40Z

reana_job_controller/app.py

        This resource is expecting JSON data with all the necessary
        information of a new job.
+      description: Create a new Job.


Creates a new job.

(BTW summary vs description to be discussed live...)

tiborsimko · 2017-05-15T08:48:23Z

reana_job_controller/app.py

+         required: true
+         schema:
+           $ref: '#/definitions/JobRequest'
+      responses:


201: Request succeeded. The job has been launched.

400: Request failed. The incoming data specification seems malformed.

500: Request failed. Internal controller error. The job could probably not have been allocated.

* Adds CLI to generate OpenAPI spec out of docstrings. * Adds initial test to validate spec. * Adds Marshmallow validation for `/jobs` POST endpoint. Signed-off-by: Diego Rodriguez <diego.rodriguez@cern.ch>

diegodelemos added Need: information labels May 5, 2017

diegodelemos force-pushed the openapi-marshmallow-integration branch 8 times, most recently from 850a222 to 3cdf4d7 Compare May 11, 2017 14:53

diegodelemos requested a review from tiborsimko May 11, 2017 14:56

tiborsimko requested changes May 12, 2017

View reviewed changes

tiborsimko mentioned this pull request May 12, 2017

api: add logging #41

Merged

diegodelemos force-pushed the openapi-marshmallow-integration branch 4 times, most recently from 339e468 to 60e7d97 Compare May 12, 2017 14:32

diegodelemos removed Need: information labels May 12, 2017

tiborsimko reviewed May 15, 2017

View reviewed changes

reana_job_controller/app.py Outdated

"status": "started"

---

get:

description: Get all Jobs

Copy link

Member

tiborsimko May 15, 2017

Choose a reason for hiding this comment

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

Returns list of all active jobs.

tiborsimko reviewed May 15, 2017

View reviewed changes

diegodelemos force-pushed the openapi-marshmallow-integration branch from 60e7d97 to e9b7dad Compare May 15, 2017 09:20

tiborsimko assigned diegodelemos May 15, 2017

diegodelemos force-pushed the openapi-marshmallow-integration branch 2 times, most recently from 044d2f6 to e3dda5b Compare May 15, 2017 12:34

diegodelemos assigned tiborsimko and unassigned diegodelemos May 15, 2017

tiborsimko approved these changes May 15, 2017

View reviewed changes

diegodelemos force-pushed the openapi-marshmallow-integration branch from e3dda5b to 8f0b5d2 Compare May 15, 2017 12:49

api: openapi specification generation

8f0b5d2

* Adds CLI to generate OpenAPI spec out of docstrings. * Adds initial test to validate spec. * Adds Marshmallow validation for `/jobs` POST endpoint. Signed-off-by: Diego Rodriguez <diego.rodriguez@cern.ch>

tiborsimko merged commit 8f0b5d2 into reanahub:master May 15, 2017

tiborsimko removed the Status: in review label May 15, 2017

diegodelemos mentioned this pull request May 28, 2017

docs: initial release reanahub/reana-server#4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: openapi specification generation #40

api: openapi specification generation #40

diegodelemos commented May 5, 2017

tiborsimko May 12, 2017

diegodelemos May 12, 2017

tiborsimko May 12, 2017

tiborsimko May 12, 2017

tiborsimko May 12, 2017

diegodelemos May 12, 2017

tiborsimko May 12, 2017

diegodelemos commented May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017

tiborsimko May 15, 2017


		import json

		import click

api: openapi specification generation #40

api: openapi specification generation #40

Conversation

diegodelemos commented May 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegodelemos commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment