Skip to content

Conversation

@Soonmok
Copy link
Contributor

@Soonmok Soonmok commented Jan 4, 2023

related: #23539

Fix xfail test in tests/jobs/test_backfill_job.py::test_trigger_controller_dag test

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Jan 4, 2023
Copy link
Member

Choose a reason for hiding this comment

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

Instead of relying on get_last_dagrun() (which can have bugs), it could be better to use SQLAlchemy directly here. There is a session pytest fixture you can use for this.

@uranusjr
Copy link
Member

uranusjr commented Jan 5, 2023

Also, could you do some history digging (git blame) to find when the xfail was added, and if there’s any reasoning associated with the addition.

@Soonmok
Copy link
Contributor Author

Soonmok commented Jan 5, 2023

Also, could you do some history digging (git blame) to find when the xfail was added, and if there’s any reasoning associated with the addition.

#8015 PR was related

@Soonmok Soonmok requested a review from uranusjr January 5, 2023 10:57
@potiuk
Copy link
Member

potiuk commented Jan 17, 2023

Static checks need fixing/rebase is needed.

@potiuk
Copy link
Member

potiuk commented Jan 18, 2023

Needs rebase/fixing static checks,

@Taragolis Taragolis added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) type:misc/internal Changelog: Misc changes that should appear in change log labels Feb 18, 2023
@Taragolis Taragolis merged commit d203000 into apache:main Feb 18, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 18, 2023

Awesome work, congrats on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Scheduler including HA (high availability) scheduler changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants