Skip to content

Conversation

@ashb
Copy link
Member

@ashb ashb commented Dec 7, 2022

The primary driver for this was a niggle that the durations output for
one test was reporting over 52 years:

1670340356.40s call tests/jobs/test_base_job.py::TestBaseJob::test_heartbeat

It turns out this was caused by freezegun, but time_machine fixes this.
It also might be a bit faster, but that isn't a noticeable difference for
us. (No runtime difference for the changed files, but it does make
collection quicker: from 10s to 8s)

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:core-operators area:dev-tools area:logging area:providers area:Scheduler including HA (high availability) scheduler provider:amazon AWS/Amazon - related issues provider:google Google (including GCP) related issues labels Dec 7, 2022
@ashb ashb removed provider:google Google (including GCP) related issues provider:amazon AWS/Amazon - related issues area:Scheduler including HA (high availability) scheduler area:providers area:logging area:API Airflow's REST/HTTP API area:core-operators labels Dec 7, 2022
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Love it. I've read https://github.com/adamchainz/time-machine and it's a cool, nice, small library. Much better and more versatile than freezegun, and I love the time_travel metapherore much more than the freezegun's one (who needs guns).

Kudos to the author for Delorean example in the docs in case you missed it.

@ashb
Copy link
Member Author

ashb commented Dec 7, 2022

Ci no likey! Local was fine. Curious.

@potiuk
Copy link
Member

potiuk commented Dec 7, 2022

My guess is some tick=False missing ? The way it runs on CI is that it is "heavy" i.e. things per process run slower even if altogether they run faster, so 1-second time ticks might happen more frequently than when running single tests.

@ashb
Copy link
Member Author

ashb commented Dec 7, 2022

It's an odd one: This combo of tests fails locally:

tests/task/task_runner/test_task_runner.py::TestGetTaskRunner::test_should_support_core_task_runner tests/sensors/test_external_task_sensor.py::TestExternalTaskSensor::test_external_task_sensor_failed_states_as_success

    @mock.patch("airflow.task.task_runner.base_task_runner.subprocess")
    @mock.patch("airflow.task.task_runner._TASK_RUNNER_NAME", "StandardTaskRunner")
    def test_should_support_core_task_runner(self, mock_subprocess):
        ti = mock.MagicMock(map_index=-1, run_as_user=None)
        ti.get_template_context.return_value = {"ti": ti}
        ti.get_dagrun.return_value.get_log_template.return_value.filename = "blah"
        local_task_job = mock.MagicMock(task_instance=ti)
        task_runner = get_task_runner(local_task_job)

        assert "StandardTaskRunner" == task_runner.__class__.__name__

I don't quite know what in that test is leaking state either!

@ashb
Copy link
Member Author

ashb commented Dec 7, 2022

OH! Main is broken with that test! Which you've just reverted :D

@ashb ashb force-pushed the freezegun-to-time-machine branch from ceee216 to 10b311e Compare December 12, 2022 14:01
The primary driver for this was a niggle that the durations output for
one test was reporting over 52 years:

> 1670340356.40s call     tests/jobs/test_base_job.py::TestBaseJob::test_heartbeat

It turns out this was caused by freezegun, but time_machine fixes this.
It also might be a bit faster, but that isn't a noticeable difference for
us. (No runtime difference for the changed files, but it does make
collection quicker: from 10s to 8s)
@ashb ashb force-pushed the freezegun-to-time-machine branch from 10b311e to b6aee61 Compare December 12, 2022 16:14
@ashb ashb merged commit 4d0fd8e into apache:main Dec 12, 2022
@ashb ashb deleted the freezegun-to-time-machine branch December 12, 2022 17:38
@pierrejeambrun pierrejeambrun added this to the Airflow 2.5.1 milestone Jan 9, 2023
@pierrejeambrun pierrejeambrun 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 and removed changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) labels Jan 9, 2023
ephraimbuddy pushed a commit that referenced this pull request Mar 8, 2023
The primary driver for this was a niggle that the durations output for
one test was reporting over 52 years:

> 1670340356.40s call     tests/jobs/test_base_job.py::TestBaseJob::test_heartbeat

It turns out this was caused by freezegun, but time_machine fixes this.
It also might be a bit faster, but that isn't a noticeable difference for
us. (No runtime difference for the changed files, but it does make
collection quicker: from 10s to 8s)

(cherry picked from commit 4d0fd8e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools 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.

5 participants