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

Move read only property in order to fix Dagrun API docs #30149

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

SamWheating
Copy link
Contributor

Closes: #30075

Note: We should either merge this or #30113, but not both.

Using $ref alongside other attributes can lead to unintentional errors in the API documentation (source):

Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at.

Because of this, the state field was incorrectly showing up in the API docs for the trigger DagRun endpoint:

Screenshot 2023-03-16 at 9 36 37 AM

By instead these attributes under allOf:, this change removes this field from the docs, making it more consistent with the intended API behaviour:
Screenshot 2023-03-16 at 9 38 03 AM

There are a few other instances in the Spec in which $ref is used alongside another attribute which could lead to similar issues, I will fix those in a subsequent PR when I have a bit more time.

@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label Mar 16, 2023
@SamWheating SamWheating changed the title Use allOf in OpenAPI Spec to remove state field from trigger Dagrun API docs Move readONly property to fix Dagrun API docs Mar 17, 2023
@SamWheating SamWheating changed the title Move readONly property to fix Dagrun API docs Move readOnly property in order to fix Dagrun API docs Mar 17, 2023
@potiuk
Copy link
Member

potiuk commented Mar 18, 2023

I think that one looks cool @ephraimbuddy ?

@ephraimbuddy
Copy link
Contributor

I think that one looks cool @ephraimbuddy ?

Yes. Makes sense.

@ephraimbuddy ephraimbuddy merged commit e01c146 into apache:main Mar 21, 2023
@pierrejeambrun pierrejeambrun added this to the Airflow 2.5.3 milestone Mar 22, 2023
@pierrejeambrun pierrejeambrun added the type:bug-fix Changelog: Bug Fixes label Mar 22, 2023
pierrejeambrun pushed a commit that referenced this pull request Mar 23, 2023
@pierrejeambrun pierrejeambrun changed the title Move readOnly property in order to fix Dagrun API docs Move read only property in order to fix Dagrun API docs Mar 24, 2023
pierrejeambrun pushed a commit that referenced this pull request Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to set DagRun state in create Dagrun endpoint ("Property is read-only - 'state'")
4 participants