You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/06/18 15:35:45 UTC
[GitHub] [airflow] mik-laj opened a new issue #9387: Use enum for task_states/dag_states
mik-laj opened a new issue #9387:
URL: https://github.com/apache/airflow/issues/9387
Hello,
We use Python 3+, so we should consider using an enumerated type for objects that are buckets for contents This will give us better type hints.
https://github.com/apache/airflow/blob/master/airflow/utils/state.py
Best regards,
Kamil Breguła
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andrewgodwin edited a comment on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
andrewgodwin edited a comment on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-814363281
I would like to ask, though - do we just want the type hinting here (in which case we can just declare a nice Literal type for the state tuples), or take on the problems that changing a tuple to an enum will cause? There's a few places in Airflow, and presumably in some third-party code, that presume they can do operations on `State.task_states` as if it were a sliceable/iterable.
(and also SQLAlchemy will not like the Enum values, resulting in a lot of casting back to string)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] muyajil commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
muyajil commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-646338044
Hi,
I'd like to help out with this issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-814997968
I briefly read related source, and it seems to me the main advantage would be to make sure the right values are passed the right value to SQLAlchemy, so `Literal` feels like a better solution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andrewgodwin commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
andrewgodwin commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-815958718
Yeah, after having done some exploratory coding here, it's basically going to require a big separate Literal type that probably duplicates a lot of the values. I'll draw it up, but we can see if it's worth it in the PR discussion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr edited a comment on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-814997968
I briefly read related source, and it seems to me the main advantage would be to make sure the right values are passed the right value to SQLAlchemy, so `Literal` feels like a better solution. `Enum` may even be a bit awkward in some places, e.g.
https://github.com/apache/airflow/blob/44a6648fd7482f61ca59153f0caaf0b019c5b3fd/airflow/models/dagrun.py#L490
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] muyajil commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
muyajil commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-812457152
@chutcheson Yes it is, unfortunately I never got the chance to really get started
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andrewgodwin commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
andrewgodwin commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-814350379
I'll take this up, if someone wants to assign me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] chutcheson commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
chutcheson commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-812223959
Is this still an open issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-646513946
@muyajil I assigned you to this ticket. I look forward to your contributions 😻
@turbaszek I think we can assign people without confirmation. The assignment of people is important because it is visible on the ticket list. This makes finding free tickets easier.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-646107470
CC: @turbaszek
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] andrewgodwin commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
andrewgodwin commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-814363281
I would like to ask, though - do we just want the type hinting here (in which case we can just declare a nice Literal type for the state tuples), or take on the problems that changing a tuple to an enum will cause? There's a few places in Airflow, and presumably in some third-party code, that presume they can do operations on `State.task_states` as if it were a sliceable/iterable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] turbaszek commented on issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
turbaszek commented on issue #9387:
URL: https://github.com/apache/airflow/issues/9387#issuecomment-646511037
@muyajil should I assign you?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil closed issue #9387: Use enum for task_states/dag_states
Posted by GitBox <gi...@apache.org>.
kaxil closed issue #9387:
URL: https://github.com/apache/airflow/issues/9387
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org