Add OpenAPI specification (II) #8721

mik-laj · 2020-05-05T19:03:45Z

Hello,

This is my second proposal for the API specification (first: #7549). The previous one was merged without acceptance. I am very sorry for this very not cool behaviour. Now based on this change proposes specifications again. I will now try to introduce changes more transparently. I have created a lot of changes that are responses to comments on the previous version. I will now try to add reference to commit for each comment.

Preview:
Swaagger UI:
https://editor.swagger.io/?url=https://raw.githubusercontent.com/PolideaInternal/airflow/openapi-spec/openapi.yaml
Redoc:
https://redocly.github.io/redoc/?url=https://raw.githubusercontent.com/PolideaInternal/airflow/openapi-spec/openapi.yaml#operation/getConfig

Best regards,
Kamil

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

Description above provides context of the change
Unit tests coverage for changes (not needed for documentation changes)
Target Github ISSUE in description if exists
Commits follow "How to write a good git commit message"
Relevant documentation is updated including usage instructions.
[xI will engage committers as explained in Contribution Workflow Example.

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.

We do not have endpoints to download jobs, because this is an implementation detail, so this field has big no value.

mik-laj · 2020-05-28T15:18:08Z

Errors

Semantic error at paths./dags/{dag_id}.get.requestBody
GET operations cannot have a requestBody.
Jump to line 181

I am fixing it. I also checked if the problem occurs elsewhere.

yq r openapi.yaml  --tojson | jq '.paths |to_entries|map(select(.value.get.requestBody))'
[]

mik-laj · 2020-05-28T15:30:36Z

@ashb I answered all comments. Would you like to add something else? 😸

ashb · 2020-05-28T15:48:40Z

Only outstanding point is about #8721 (comment) (conf param to trigger as string vs object) -- I'll take it to the mailing list.

ashb · 2020-05-28T21:58:06Z

Posted to https://lists.apache.org/thread.html/r88f5f57b4586f75eda382ce2c7309bd32b58cd9cfa1b01f681b6c8d1%40%3Cdev.airflow.apache.org%3E

potiuk

Need to change key to xcom_key - it's too generic and causes Java automatically generated client to fail

potiuk · 2020-05-30T18:25:03Z

openapi.yaml

+
+    XComKey:
+      in: path
+      name: key


Suggested change

name: key

name: key

Suggested change

name: key

name: xcom_key

I'm not a fan of this really -- what specifically is the problem this causes for Java?

Here are a related change. (I did it before I read your message.) We can revert it if we make a different decision.
5993aef

Generated code of the method uses internally "key" variable to loop through the parameters. In Java you cannot have same variable and parameter name.

I will be happy if you find any other solution for that.

Code here: https://github.com/mik-laj/airflow-api-clients/blob/master/out/object/java-google-api-client/src/main/java/org/openapitools/client/api/XComApi.java#L108 (now the parameter is named xcomkey - it used to be named key)

And BTW. this is the main reason why I created #9080 and I am super keen on automating the checks if the main language open-api clients can be automatically generated and compiled. Sometimes thing like choosing a name might matter (of course it should not matter in theory but we are leaving in an imperfect world). Making the clients generated and being usable is a priority for me in this case,

Code here: https://github.com/mik-laj/airflow-api-clients/blob/master/out/object/java-google-api-client/src/main/java/org/openapitools/client/api/XComApi.java#L108 (now the parameter is named xcomkey - it used to be named key)

It didn't cause any problems in my compile tests...

Ah. I tested with the java-okhttp-gson, and it doesn't have that problem.

So next question: do we want to/need support every possible API generator? that's a lot of combinations to build+test+support?

Have we reported this bug upstream too?

openapi.yaml

potiuk · 2020-05-31T10:21:49Z

openapi.yaml

+          default: true
+          readOnly: True
+        conf:
+          type: string


Suggested change

type: string

type: object

mik-laj · 2020-06-01T14:52:57Z

Can I still help in finishing this change? I think that the discussion between xcom_key and key is not a big issues, and even goes beyond the scope of this change. We have learned that in addition to the OpenAPi 3.0 specification it is worth considering other requirements and they will definitely affect the final specifications during the following months.

potiuk · 2020-06-01T15:20:57Z

I think we should submit with xcom_key as we know that it at least compile in open-api generator. So I suggest we merge it as is. @ashb -> I think the two interns are pretty much blocked by this change and they could start working on it. we can always make a follow-up change later on if we find a better solution. I want to make the follow-up CI change soon as well so that any future change/proposal will get automatically tested.

potiuk

LGTM

ashb · 2020-06-01T18:49:03Z

Yeah not a fan of xcomKey vs key but not a blocker from me.

ashb · 2020-06-01T18:55:13Z

openapi.yaml

+
+    # Common data type
+    ScheduleInterval:
+      allOf:


This one should be oneOf

Thanks. I updated the spec.
6153d5b

ashb · 2020-06-01T19:23:22Z

Curious, wonder why Mergeable is failing here...

kaxil · 2020-06-01T19:24:26Z

Curious, wonder why Mergeable is failing here...

Because of "dont-merge" label

ashb · 2020-06-01T19:26:59Z

🎉

mik-laj · 2020-06-01T19:33:48Z

The longest discussion 🐈

Thank you all for a great job. This is just one file and the beginning of implementation.

houqp · 2020-06-02T01:18:22Z

thanks @mik-laj for driving this and congrats everyone for a big milestone :)

turbaszek · 2020-06-02T07:23:18Z

Thanks @mik-laj ! 🚀

mik-laj added the area:API Airflow's REST/HTTP API label May 5, 2020

boring-cyborg bot added the area:dev-tools label May 5, 2020

mik-laj and others added 14 commits May 5, 2020 21:04

Add OpenAPI spec (apache#7549)

d9a154e

Fix typo in name of pre-commit hook

c268395

Chaange type for DAGID, DAGRunID, TaskID

cd2be1c

Fix typo in summary - POST /pools

ce40ff5

Fix typo in description - FileToken parameter

47d779e

Fix typo - singular/plural form - variables

af260cb

Make EventLog endpoints read-only

d7d0ca5

Use ExcutionDate in DagRuns endpoints

3d014d6

Use custom action to control task instances

6514d68

Typo in DELETE Task instance

4f99a1f

Remove unused schema - DagStructureCollection

9006d3d

Fix typo - singular/plural form - import errors

69e769d

Add endpoint - POST /dagRuns

00af377

Remove job_id

aec2275

We do not have endpoints to download jobs, because this is an implementation detail, so this field has big no value.

mik-laj force-pushed the openapi-spec branch 2 times, most recently from 9b6a87c to f0d53f4 Compare May 5, 2020 19:11

Kamil Breguła added 3 commits May 5, 2020 21:14

Add filters to GET /taskInstances

204928a

Fix typo - upadtePool => updatePool

b661d0c

Rename "Create a DAG Run" to "Trigger a DAG Run"

0764b82

mik-laj force-pushed the openapi-spec branch from f0d53f4 to 0764b82 Compare May 5, 2020 19:15

mik-laj mentioned this pull request May 5, 2020

Add OpenAPI spec #7549

Merged

6 tasks

Kamil Breguła added 7 commits May 5, 2020 21:27

Use Pool name as a parameter

baa22d7

Add filter to GET /dagRuns

d922f2b

Remove invalid note ion start_date field

221bedb

Uss POST instead of PATCH for custom action

932d115

Remove DELETE /taskInstances endpooint

acf6e2d

Rename Xcom Value to xcom Entry

26e0d22

Fix typo in XCCOM Entry endpoint

eded243

Remove request body for GET endpoint

6216cab

mik-laj requested a review from ashb May 28, 2020 15:29

potiuk reviewed May 30, 2020

View reviewed changes

potiuk reviewed May 31, 2020

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

potiuk reviewed May 31, 2020

View reviewed changes

Kamil Breguła added 4 commits June 1, 2020 13:59

Use allOf insteaad of oneOf

31d8e15

Rename key => xcom_key

5993aef

Use lowercase letters in query string parameter - Queue -> queue

7bbfa4a

Change type of conf to object

c5456d7

potiuk approved these changes Jun 1, 2020

View reviewed changes

ashb requested changes Jun 1, 2020

View reviewed changes

Change allOf into oneOf for ScheduleInterval

6153d5b

kaxil removed the dont-merge label Jun 1, 2020

ashb approved these changes Jun 1, 2020

View reviewed changes

mik-laj merged commit 20f8982 into apache:master Jun 1, 2020

mik-laj deleted the openapi-spec branch June 1, 2020 19:46

potiuk mentioned this pull request Oct 9, 2020

Fully support running more than one scheduler concurrently #10956

Merged

8 tasks

Add OpenAPI specification (II) #8721

Add OpenAPI specification (II) #8721

Uh oh!

Conversation

mik-laj commented May 5, 2020 • edited by kaxil Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mik-laj commented May 28, 2020

Uh oh!

mik-laj commented May 28, 2020

Uh oh!

ashb commented May 28, 2020

Uh oh!

ashb commented May 28, 2020

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mik-laj commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

potiuk commented Jun 1, 2020

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

ashb commented Jun 1, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashb commented Jun 1, 2020

Uh oh!

kaxil commented Jun 1, 2020

Uh oh!

ashb commented Jun 1, 2020

Uh oh!

mik-laj commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

houqp commented Jun 2, 2020

Uh oh!

turbaszek commented Jun 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

mik-laj commented May 5, 2020 •

edited by kaxil

Loading

mik-laj commented Jun 1, 2020 •

edited

Loading

mik-laj commented Jun 1, 2020 •

edited

Loading