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

Add DagModel attributes before dumping DagDetailSchema for get_dag_details API endpoint #34947

Merged

Conversation

RNHTTR
Copy link
Contributor

@RNHTTR RNHTTR commented Oct 14, 2023

Closes: #33482


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added the area:API Airflow's REST/HTTP API label Oct 14, 2023
@RNHTTR RNHTTR changed the title Add DagModel attributes before dumping DagDetailSchema Add DagModel attributes before dumping DagDetailSchema for get_dag_details API endpoint Oct 16, 2023
@RNHTTR RNHTTR marked this pull request as ready for review October 16, 2023 22:51
@RNHTTR RNHTTR force-pushed the add-missing-data-to-get-dag-details-endpoint branch from eda5e8e to 6f2c73c Compare October 23, 2023 00:28
@RNHTTR RNHTTR force-pushed the add-missing-data-to-get-dag-details-endpoint branch from 6f2c73c to ec52276 Compare November 9, 2023 17:21
@RNHTTR RNHTTR requested a review from uranusjr November 9, 2023 22:41
Copy link
Member

Choose a reason for hiding this comment

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

Are quote changes in this file needed? They are obscuring the actual addition…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by quote changes

Copy link
Member

@pierrejeambrun pierrejeambrun Nov 22, 2023

Choose a reason for hiding this comment

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

There are an important number of lines changed located in the OpenAPI spec. All simple quotes are moved to double quotes.

I also agree that it makes the addition harder to review, I suggest we do this in a separate PR (I don't think it is needed in this particular PR).

Makes me realize that we do not have yamllint activated on our OpenAPI Spec to enforce common style.

@uranusjr
Copy link
Member

A couple of minor questions, otherwise lgtm.

@eladkal eladkal added this to the Airflow 2.8.0 milestone Nov 24, 2023
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Nov 24, 2023
@eladkal eladkal merged commit e8f62e8 into apache:main Nov 24, 2023
47 checks passed
ephraimbuddy pushed a commit that referenced this pull request Nov 26, 2023
…tails API endpoint (#34947)

* Add DagModel attributes before dumping DagDetailSchema

* fix tests

* fix tests

* missed an f-string :(

* apply dag attributes to dag model instead of other way around

* pass session to
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.

The /dags/{dag_id}/details endpoint returns less data than is documented
4 participants