Skip to content

Replace State usages with strong-typed enums#31735

Merged
dstandish merged 7 commits into
apache:mainfrom
astronomer:use-strong-typed-state-enums
Jun 16, 2023
Merged

Replace State usages with strong-typed enums#31735
dstandish merged 7 commits into
apache:mainfrom
astronomer:use-strong-typed-state-enums

Conversation

@uranusjr

@uranusjr uranusjr commented Jun 6, 2023

Copy link
Copy Markdown
Member

Only in the main Airflow code base. There are many more in tests that I might tackle some day.

@boring-cyborg boring-cyborg Bot added area:API Airflow's REST/HTTP API area:CLI area:core-operators provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:Scheduler including HA (high availability) scheduler labels Jun 6, 2023
@uranusjr
Replace State usages with strong-typed enums
3d50998 
Only in the main Airflow code base. There are many more in tests that I
might tackle some day.
@uranusjr
uranusjr force-pushed the use-strong-typed-state-enums branch from 2f695f6 to 3d50998 Compare June 6, 2023 08:59
dstandish
dstandish reviewed Jun 6, 2023
View reviewed changes
Comment thread airflow/cli/commands/jobs_command.py Outdated
dstandish
dstandish reviewed Jun 6, 2023
View reviewed changes
Comment thread airflow/jobs/job.py Outdated
dstandish
dstandish reviewed Jun 6, 2023
View reviewed changes
Comment thread airflow/jobs/job.py Outdated
dstandish
dstandish approved these changes Jun 7, 2023
View reviewed changes

@dstandish dstandish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

up to you, sir

jedcunningham
jedcunningham reviewed Jun 7, 2023
View reviewed changes
Comment thread airflow/cli/commands/jobs_command.py Outdated
Comment thread airflow/jobs/scheduler_job_runner.py Outdated
Comment thread airflow/jobs/scheduler_job_runner.py Outdated
Comment thread airflow/jobs/scheduler_job_runner.py Outdated
Comment thread airflow/kubernetes/pod_launcher_deprecated.py Outdated
Comment thread airflow/sentry.py Outdated
@uranusjr

uranusjr commented Jun 8, 2023

Copy link
Copy Markdown
Member Author

I’m going to revert the Job-related changes and introduce JobState in a separate PR.

@dstandish
dstandish merged commit b459af3 into apache:main Jun 16, 2023
@dstandish
dstandish deleted the use-strong-typed-state-enums branch June 16, 2023 20:48
potiuk added a commit to potiuk/airflow that referenced this pull request Jun 17, 2023
@potiuk
Revert "Replace State usages with strong-typed enums (apache#31735)"
7f845a9 
This reverts commit b459af3.
potiuk added a commit that referenced this pull request Jun 17, 2023
@potiuk
Revert "Replace State usages with strong-typed enums (#31735)" (#31981)
546588a 
This reverts commit b459af3.
@potiuk

potiuk commented Jun 17, 2023

Copy link
Copy Markdown
Member

Hey @uranusjr :

I reverted this one in #31981. We need to take a close look and when it is re-done, "full test needed label" should be applied

Kubernetes tests started to fail when it got merged and when you look to scheduler logs (they are attached as artifacts to failed jobs). The error indicates it's the None Enum

See https://github.com/apache/airflow/actions/runs/5297252493/jobs/9589070710

And https://github.com/apache/airflow/actions/runs/5297252493 you can download kind-logs-k8s artifact.

When you drill down for example to pods/scheduler logs you will see:

2023-06-17T09:20:21.014975825Z stdout F [2023-06-17T09:20:21.014+0000] {kubernetes_executor.py:707} INFO - Changing state of (TaskInstanceKey(dag_id='example_kubernetes_executor', task_id='other_namespace_task', run_id='manual__2023-06-17T09:07:35.849355+00:00', try_number=1, map_index=-1), None, 'example-kubernetes-executor-other-namespace-task-i9qcs639', 'airflow', '1925') to None
2023-06-17T09:20:21.014986528Z stdout F [2023-06-17T09:20:21.014+0000] {kubernetes_executor.py:711} ERROR - Exception: None is not a valid TaskInstanceState when attempting to change state of (TaskInstanceKey(dag_id='example_kubernetes_executor', task_id='other_namespace_task', run_id='manual__2023-06-17T09:07:35.849355+00:00', try_number=1, map_index=-1), None, 'example-kubernetes-executor-other-namespace-task-i9qcs639', 'airflow', '1925') to None, re-queueing.
2023-06-17T09:20:21.014988629Z stdout F Traceback (most recent call last):
2023-06-17T09:20:21.014990179Z stdout F   File "/home/airflow/.local/lib/python3.8/site-packages/airflow/executors/kubernetes_executor.py", line 709, in sync
2023-06-17T09:20:21.014991743Z stdout F     self._change_state(key, TaskInstanceState(state), pod_name, namespace)
2023-06-17T09:20:21.01499339Z stdout F   File "/usr/local/lib/python3.8/enum.py", line 339, in __call__
2023-06-17T09:20:21.014994835Z stdout F     return cls.__new__(cls, value)
2023-06-17T09:20:21.015001917Z stdout F   File "/usr/local/lib/python3.8/enum.py", line 663, in __new__
2023-06-17T09:20:21.015004227Z stdout F     raise ve_exc
2023-06-17T09:20:21.015005646Z stdout F ValueError: None is not a valid TaskInstanceState
```

@ephraimbuddy ephraimbuddy added this to the Airlfow 2.6.3 milestone Jul 6, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Jul 6, 2023
@ephraimbuddy ephraimbuddy removed this from the Airlfow 2.6.3 milestone Jul 6, 2023
@ephraimbuddy ephraimbuddy removed the type:misc/internal Changelog: Misc changes that should appear in change log label Jul 6, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.0 milestone Aug 1, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dstandish dstandish dstandish approved these changes
@jedcunningham jedcunningham jedcunningham approved these changes
@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton
@ashb ashb Awaiting requested review from ashb
@bbovenzi bbovenzi Awaiting requested review from bbovenzi
@pierrejeambrun pierrejeambrun Awaiting requested review from pierrejeambrun
@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy
@kaxil kaxil Awaiting requested review from kaxil
@XD-DENG XD-DENG Awaiting requested review from XD-DENG
@o-nikolas o-nikolas Awaiting requested review from o-nikolas

Assignees

No one assigned

Labels

area:API Airflow's REST/HTTP API area:CLI area:Scheduler including HA (high availability) scheduler provider:cncf-kubernetes Kubernetes (k8s) provider related issues type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Milestone

Airflow 2.7.0

Development

Successfully merging this pull request may close these issues.

5 participants

@uranusjr @potiuk @dstandish @jedcunningham @ephraimbuddy