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

Mark test_process_sigterm_works_with_retries quarantined again #29075

Merged
merged 1 commit into from
Jan 21, 2023

Conversation

Taragolis
Copy link
Contributor

Unfortunately this test still not stable and first run 🤦‍♂️ 🤦‍♂️ 🤦‍♂️ in main failed again.

https://github.com/apache/airflow/actions/runs/3971462849/jobs/6808560333:

        finally:
            proc.kill()
    
        assert retry_callback_called.value == 1
        with create_session() as session:
            ti = (
                session.query(TaskInstance)
                .filter(
                    TaskInstance.dag_id == dag_id,
                    TaskInstance.task_id == task_id,
                    TaskInstance.run_id == run_id,
                )
                .one()
            )
>       assert ti.state == State.UP_FOR_RETRY
E       AssertionError: assert 'running' == <TaskInstance...up_for_retry'>
E         - up_for_retry
E         + running

tests/jobs/test_local_task_job.py:895: AssertionError

This error basically mean that we send SIGKILL proc.kill() before state changed but callback already executed.

Note: We do not check state in the previous version of the test, only callback result

@Taragolis Taragolis requested a review from potiuk January 20, 2023 23:41
@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Jan 20, 2023
@Taragolis Taragolis mentioned this pull request Jan 20, 2023
7 tasks
@Taragolis Taragolis added Quarantine Issues that are occasionally failing and are quarantined type:misc/internal Changelog: Misc changes that should appear in change log labels Jan 20, 2023
@Taragolis
Copy link
Contributor Author

Taragolis commented Jan 20, 2023

Oh... status check not make any sense, we initially call all callbacks and only after that we commit to DB.

if task and email_for_state(task) and task.email:
try:
self.email_alert(error, task)
except Exception:
self.log.exception("Failed to send email to: %s", task.email)
if callbacks and context:
self._run_finished_callback(callbacks, context, callback_type)
if not test_mode:
session.merge(self)
session.flush()

@potiuk potiuk merged commit 8eb3489 into apache:main Jan 21, 2023
maggesssss pushed a commit to maggesssss/airflow that referenced this pull request Jan 21, 2023
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 Quarantine Issues that are occasionally failing and are quarantined 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.

2 participants