Add OpenAPI spec #7549

mik-laj · 2020-02-26T16:05:14Z

I prepared an OpenAPI spec that contains the following elements:

endpoints
scheme
response bodies

Subsequent PR may extend the Open API spec of subsequent elements, e.g. HATEOAS.

Make sure to mark the boxes below before creating PR: [x]

Description above provides context of the change
Commit message/PR title starts with [AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID^*
Unit tests coverage for changes (not needed for documentation changes)
Commits follow "How to write a good git commit message"
Relevant documentation is updated including usage instructions.
I will engage committers as explained in Contribution Workflow Example.

^* For document-only changes commit message can start with [AIRFLOW-XXXX].

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

openapi.yaml

ad-m · 2020-02-27T01:16:52Z

I like the direction of the changes. I have no comments on the document structure (it is correct), nor on the presented project of the API structure (it is correct in my opinion). The only minor remarks about the standardization of formatting the specification.

openapi.yaml

ad-m · 2020-03-06T19:40:14Z

@mik-laj , the changes look good.

@nuclearpinguin, I don't know the full process of accepting changes to this project. What can we do to go further with this?

turbaszek · 2020-03-07T10:16:08Z

@nuclearpinguin, I don't know the full process of accepting changes to this project. What can we do to go further with this?

@ad-m we've got to wait for the outcome of voting on this change (it's going to happen soon).

ad-m · 2020-03-07T10:22:19Z

@nuclearpinguin Where will this voting take place? Do you have significant arguments against adopting this changes or are you going to vote in favor of the changes?

turbaszek · 2020-03-07T10:41:40Z

@ad-m for arguments and the whole discussion please check this mailing list thread (as well as AIP mentioned there):
https://lists.apache.org/thread.html/r2a0d0fb3d4610432fa52148d7d9e59c7632dd8f2fa61a580430b814c%40%3Cdev.airflow.apache.org%3E

The voting will also take place on Airflow dev mailing list:
https://lists.apache.org/[email protected]

mik-laj · 2020-04-04T15:46:27Z

This PR is part of AIP-32. It was discussed and voted on the mailing list.
https://lists.apache.org/thread.html/rb46b10dbeb96292f612461f1ed79777ef37be24d88d1ce3179c21fab%40%3Cdev.airflow.apache.org%3E
..so I merge it. We can make further changes in separate PRs.

dlamblin · 2020-04-06T14:37:41Z

I ended up dropping a comment on the wrong PR about this… PolideaInternal#653 (comment)
You did fix most every typo, but there's still "Encryption and encryption" one case of "aa", another of "a variables" and "an import errors". Otherwise this file was really handy as a reference and I'm looking into how I can use it for a system that takes spec version 2.

I did have a question about what happened to triggering a DAG by creating a DAG run. Perhaps I am just missing it?

mik-laj · 2020-04-06T15:04:04Z

You did fix most every typo, but there's still "Encryption and encryption" one case of "aa", another of "a variables" and "an import errors".

Can you create PR with your change suggestions? Unfortunately, manually comparing differences is very problematic.

I did have a question about what happened to triggering a DAG by creating a DAG run. Perhaps I am just missing it?

If DAG Run is created and the conditions for the tasks to be started are met, tasks will be started. I added your question to corresponding issue: #8120

ashb · 2020-04-08T16:06:01Z

@mik-laj Merging any PR without an approval is very Not Cool. Please don't do that.

ashb · 2020-04-08T16:12:33Z

openapi.yaml

+      in: path
+      name: dag_id
+      schema:
+        type: integer


@mik-laj You didn't fix this like you said you did.

I created new PR with updated specification: #8721

Here is the commit that makes the change.
cd2be1c

ashb · 2020-04-08T16:13:06Z

openapi.yaml

+      in: path
+      name: task_id
+      schema:
+        type: integer


I created new PR with updated specification: #8721

Here is the commit that makes the change.
cd2be1c

ashb · 2020-04-08T16:14:11Z

openapi.yaml

+      in: path
+      name: pool_id
+      schema:
+        type: integer


This is wrong too. The identifier that we should use is the pool name, it's unique (i.e. a string) not the never exposed integer id.

I created new PR with updated specification: #8721

Here is the commit that makes the change.
baa22d7

Can you look if everything is alright?

This reverts commit 11f1e0c. The OpenAPI spec as added in has problems and needs revisiting (typos, wrong IDs, wrong types)

dlamblin

I started by trying to identify the minor typos I spotted, but in detail I found a question about trigger dags, clearing tasks, and the values of xcom entries.

dlamblin · 2020-04-09T03:28:23Z

openapi.yaml

+          $ref: '#/components/responses/PermissionDenied'
+
+    post:
+      summary: Create aa pool


This is the "aa" typo I would like fixed.

Good point.

I created new PR with updated specification: #8721

Here is the commit that makes the change.
ce40ff5

Can you look if everything is alright?

dlamblin · 2020-04-09T03:29:34Z

openapi.yaml

+        type: string
+      required: true
+      description: >
+        The key containing the encrypted path to the file. Encryption and encryption takes place


I believe you meant to write: Encryption and decryption take place only on the server side.
or: Encryption takes place only on the server side.

Thanks.
Here is commit with the change: 47d779e

dlamblin · 2020-04-09T03:30:29Z

openapi.yaml

+          $ref: '#/components/responses/PermissionDenied'
+
+    post:
+      summary: Create a variables


It appears to only make one variable: Create a variable

Thanks.
Here is commit with the change: af260cb

dlamblin · 2020-04-09T03:31:29Z

openapi.yaml

+      - $ref: '#/components/parameters/VariableID'
+
+    get:
+      summary: Get a variables by id


And: Get a variable by id

Thanks.
Here is commit with the change: af260cb

dlamblin · 2020-04-09T03:33:57Z

openapi.yaml

+      - $ref: '#/components/parameters/ImportErrorID'
+
+    get:
+      summary: Get an import errors


Better as: Get an import error
[assuming ImportErrorIDs are unique]

Thanks
Here is commit: 69e769d

dlamblin · 2020-04-09T04:44:50Z

openapi.yaml

+        '403':
+          $ref: '#/components/responses/PermissionDenied'
+
+  /dags/{dag_id}/taskInstances/{task_id}/{execution_date}/xcomValues/{key}:


While the xcom table uses ['dag_id', 'task_id', 'key', 'execution_date'] as the primary key, the task and dag ids are the string names of those entities.

I tried to build hierarchical URLs to make these APIs easier to use. How would you like these API URLs to look?

dlamblin · 2020-04-09T04:46:48Z

openapi.yaml

+
+    patch:
+      summary: Update a pool
+      operationId: upadtePool


operationId: updatePool

Thanks. Here is commit: b661d0c

dlamblin · 2020-04-09T04:48:09Z

openapi.yaml

+
+    post:
+      summary: Create an XCom entry
+      operationId: updateXComValues


I think this should be createXComEntry

I updated spec. Here is commit: 26e0d22

dlamblin · 2020-04-09T05:29:24Z

openapi.yaml

+      description:
+        This endpoint support reading resources across multiple Task Instances by specifying a
+        "-" as a `dag_id`, `task_id` and `execution_date`.
+      operationId: getXComValues


Because the value [is/should be] just one field of the returned by XcomCollectionItem these operationIds under this path which end in Value(s) get a little confusing. Maybe we could rename them like: getXComEntries? The same can be said about the summary and description mentioning just the values.

I updated spec. Here is commit: 26e0d22

dlamblin · 2020-04-09T05:29:55Z

openapi.yaml

+    XComCollectionItem:
+      # Divided into two schemas for sensitive data protection
+      type: object
+      properties:


The XComCollectionItem appears to be missing a value property.

This is intentional, because value can be very big. This information is available in the details of the specific resource.

dlamblin · 2020-04-09T05:45:45Z

openapi.yaml

+
+    patch:
+      summary: Update a connection entry
+      operationId: patchConnection


Let's name the operationId: updateConnection. This is the only patch operation that is not named update something.

OperationID will change in the future due to integration with connexion.

Here is commit: f40f400

mik-laj · 2020-05-05T20:14:17Z

Hello,
I created a new PR with updated specification:
#8721
Best regards,
Kamil

* Add OpenAPI spec (#7549) * Fix typo in name of pre-commit hook * Chaange type for DAGID, DAGRunID, TaskID * Fix typo in summary - POST /pools * Fix typo in description - FileToken parameter * Fix typo - singular/plural form - variables * Make EventLog endpoints read-only * Use ExcutionDate in DagRuns endpoints * Use custom action to control task instances * Typo in DELETE Task instance * Remove unused schema - DagStructureCollection * Fix typo - singular/plural form - import errors * Add endpoint - POST /dagRuns * Remove job_id We do not have endpoints to download jobs, because this is an implementation detail, so this field has big no value. * Add filters to GET /taskInstances * Fix typo - upadtePool => updatePool * Rename "Create a DAG Run" to "Trigger a DAG Run" * Use Pool name as a parameter * Add filter to GET /dagRuns * Remove invalid note ion start_date field * Uss POST instead of PATCH for custom action * Remove DELETE /taskInstances endpooint * Rename Xcom Value to xcom Entry * Fix typo in XCCOM Entry endpoint * Change operationID: patchConnection => updaateConnection * Make execution_date optionall in DAGRun This field can be filled with the current date when creating the DAG Run * Unify connection ID * Use URL with HTTPS and without www. * Fix typo - at database => in database * Fix typo = Raange -> Raange * Fix typo - the specific DAG => a DAG * Fix typo - getXComVEntry => getXComVEntry * Unify collection names - xcomEntries * Move TaskInstance resource under DagRun resource * Fix typo - change tag - TaskInstance => TaskInstance Co-authored-by: Ash Berlin-Taylor <[email protected]> * Use path paramaters for /variables/lookup/ endpoint * Use consistent names for IDs * Use new style for filter parameters * Remove unused path parameter * Use ~ as a wildcard charaacter * Add batch endpoints for TaskInstance and DagRuns * Fix typo - response in trigger dag endpoint * Fix typo - Qqueue => Queue * Set dry_run = True in ClearTaskInstance * Mark all fieldss (expcet state) of DagRun as read-only * Use __type as a discriminator * Fix typo - "The Import Error ID." => "The Event Log ID." * Fix typo - Self referential in EventLogCollection * Rename fieldss - dttm => when * remove fields - pool_id * Fix typo - change request body in PATCH /pools/{pool_name} * Use DAG Run ID as a primary identifier * Fix typo - Change type of query to string * Unify fields names in collections * Use variable key as a primary id * Move collection to /variables * Mark passord as a write only * Fix typo - updaateConnection => updateConnection * Change is_paused/is_subdag to boolean * Fix typo - clearTaskInstaance => clearTaskInstance * Fix typo - DAAG => DAG * Fix typo - many => multiple * Fix typo - missing "a" * Fix typo - variable by id => variable by key * Fix typo - updateXComEntries => updateXComEntry * Fix typo - missing "a" * Use dag_run_id as a primary ID * Fix typo - objectss => objects, DAG IDS => DAG IDs * Allows create DAG Run with custom conf/execution_date/dag_run_id * Add new trigger rule, fix typo in dag run state * Add request body to POST/PATCH /dags/{dag_id} * Rename collection fields - dag_model => dags * Fix typo - /clearTaskInstanaces -> /clearTaskInstances * Improve wording - wildcard * Returns owners as a array * Return only references in clear task instances * Remove support for application/x-www-form-urlencoded * fixup! Use __type as a discriminator * Add file_token fields * Move description of variable collections * Return SUB DAG in DAG structure * Fix typo - sucess => sucess, Apache Foundation => Apache Software Foundation, Airfow => Apache Airflow * Improve description of get logs endpoint * Fix typo - Get all XCom entry => Get all XCom entries * Add crossreference between /dags/{dag_id}/structure and /dags/{dag_id} * Remove all form-urllencoded request bodies * Rename parameter - NoChunking => FullContent * Improve description of batch endpoints * Remove request body for GET endpoint * Use allOf insteaad of oneOf * Rename key => xcom_key * Use lowercase letters in query string parameter - Queue -> queue * Change type of conf to object * Change allOf into oneOf for ScheduleInterval Co-authored-by: Ash Berlin-Taylor <[email protected]>

mik-laj changed the title ~~[AIRFLOW-6929] Add OpenAPI spec~~ [AIRFLOW-6929][DONT-MERGE] Add OpenAPI spec Feb 26, 2020

mik-laj force-pushed the aip-XXXXX-api branch from 6270298 to ed650a8 Compare February 26, 2020 16:06

kaxil added the dont-merge label Feb 26, 2020

mik-laj force-pushed the aip-XXXXX-api branch 3 times, most recently from ac2f7d8 to 4f610c3 Compare February 27, 2020 01:01

ad-m suggested changes Feb 27, 2020

View reviewed changes