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

Fix - TestSchedulerJobQueriesCount::test_process_dags_queries_count #12273

Merged
merged 1 commit into from
Nov 12, 2020

Conversation

mik-laj
Copy link
Member

@mik-laj mik-laj commented Nov 11, 2020

The first time this test was broken in the following commit.
406ed29 Make Dag Serialization a hard requirement (#11335)
And the number of database queries has changed: Fortunately, it was optimization.
92e405e Call scheduler "book-keeping" operations less frequently. (#12139)

A few more tests look like they might be broken too:

with mock.patch.object(settings, "STORE_SERIALIZED_DAGS", True):

settings, 'STORE_SERIALIZED_DAGS', True

Thanks to @potiuk for creating the quarantine test list. This allowed me to see that the tests that had previously been only partially unstable. Now it was never successful.
#10118

CC: @ashb @kaxil


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
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.

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Nov 11, 2020
@mik-laj mik-laj added full tests needed We need to run full set of tests for this PR to merge and removed full tests needed We need to run full set of tests for this PR to merge labels Nov 11, 2020
@mik-laj
Copy link
Member Author

mik-laj commented Nov 12, 2020

@kaxil @ashb Can you look at it?

@ashb
Copy link
Member

ashb commented Nov 12, 2020

Will do

@kaxil
Copy link
Member

kaxil commented Nov 12, 2020

The tests were passing on #11335

@mik-laj
Copy link
Member Author

mik-laj commented Nov 12, 2020

@kaxil I am afraid that this test may not start for some reason. I tested locally and I have a message that this attribute is missing. It have been deleted. See: https://github.com/apache/airflow/pull/11335/files#diff-230a06cebe53507c57e3076e4651050de7b8e741212f304acc516a338fce3332L347-L349

@kaxil
Copy link
Member

kaxil commented Nov 12, 2020

@kaxil I am afraid that this test may not start for some reason. I tested locally and I have a message that this attribute is missing. It have been deleted. See: https://github.com/apache/airflow/pull/11335/files#diff-230a06cebe53507c57e3076e4651050de7b8e741212f304acc516a338fce3332L347-L349

Got it, yes, right

Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

I will fix the other tests where config is still there

@github-actions
Copy link

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 12, 2020
@mik-laj
Copy link
Member Author

mik-laj commented Nov 12, 2020

I checked the log for those tests on CI and these tests are now green.

tests/jobs/test_scheduler_job.py FFFFF...........................        [ 87%]

@mik-laj mik-laj merged commit 82eef2e into apache:master Nov 12, 2020
@mik-laj mik-laj deleted the fix-query-count-test branch November 12, 2020 13:40
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 full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants