Skip to content

AIP-84 Add ability to update dag run note in PATCH dag_run endpoint #43508

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.

Sign up for GitHub

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

Jump to bottom
Merged
pierrejeambrun merged 18 commits into from
Nov 8, 2024
Merged

AIP-84 Add ability to update dag run note in PATCH dag_run endpoint #43508

pierrejeambrun merged 18 commits into from
Nov 8, 2024

Conversation

@rawwar
Copy link
Contributor

@rawwar rawwar commented Oct 30, 2024

closes #43476

related to #42701

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Oct 30, 2024
@rawwar rawwar mentioned this pull request Oct 30, 2024
Closed
12 tasks
rawwar
rawwar commented Oct 30, 2024
View reviewed changes
@rawwar rawwar mentioned this pull request Oct 31, 2024
Closed
5 tasks
rawwar
rawwar commented Oct 31, 2024
View reviewed changes
@pierrejeambrun
Copy link
Member

pierrejeambrun commented Nov 5, 2024 •
edited
Loading

I just pushed a new commit that should help you. Tests are green for test_patch_dag_run and both e75afb7.

The issue is coming session related and coming from the fact that methods such as set_dag_run_state_to_success are not updating the dag_run that we have in our context. Therefore we can do that beforehand, refetch the dag_run and then proceed to normal update.

Maybe we want to play around session=session, also commit=True is not actually committing to the db but only doing the modification, that's misleading but it is actually what we need. (1 commit per request to guarantee data integrity)

pierrejeambrun
pierrejeambrun reviewed Nov 5, 2024
View reviewed changes
@rawwar rawwar marked this pull request as ready for review November 6, 2024 13:44
@rawwar rawwar requested a review from pierrejeambrun November 6, 2024 13:45
pierrejeambrun
pierrejeambrun reviewed Nov 7, 2024
View reviewed changes
@rawwar rawwar requested a review from pierrejeambrun November 7, 2024 13:51
bugraoz93
bugraoz93 reviewed Nov 7, 2024
View reviewed changes
rawwar
rawwar commented Nov 8, 2024
View reviewed changes
@pierrejeambrun
Copy link
Member

pierrejeambrun commented Nov 8, 2024

Thanks

@pierrejeambrun pierrejeambrun merged commit f57db71 into apache:main Nov 8, 2024
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
@rawwar @pierrejeambrun @ellisms
AIP-84 Add ability to update dag run note in PATCH dag_run endpoint (a…
fbaa614 
…pache#43508)

* include dag_run_note in update_mask

* add dag run note

* state can be none

* add test

* Fix tests

* handle edge cases

* add tests

* remove joinedload

* fix update_mask checks

* fix tests

* fix

* remove async

* undo async

* fix

---------

Co-authored-by: pierrejeambrun <pierrejbrun@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bugraoz93 bugraoz93 bugraoz93 left review comments

@pierrejeambrun pierrejeambrun pierrejeambrun approved these changes

@bbovenzi bbovenzi Awaiting requested review from bbovenzi

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton

@jscheffl jscheffl Awaiting requested review from jscheffl

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy

Assignees

No one assigned

Labels

area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AIP-84 Add ability to update dag run note in PATCH dag_run endpoint

3 participants

@rawwar @pierrejeambrun @bugraoz93