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 2022/08/24 16:32:13 UTC
[GitHub] [airflow] dstandish opened a new pull request, #25943: Make dataset_event_manager a cached property on TaskInstance
dstandish opened a new pull request, #25943:
URL: https://github.com/apache/airflow/pull/25943
Appears `__init__` is not invoked as part of `_run_raw_task` due to the way TI is refreshed from db. Using cached property ensures it will be there.
--
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
[GitHub] [airflow] jedcunningham merged pull request #25943: Fix dataset_event_manager resolution
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #25943:
URL: https://github.com/apache/airflow/pull/25943
--
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
[GitHub] [airflow] dstandish commented on a diff in pull request #25943: Make dataset_event_manager a cached property on TaskInstance
Posted by GitBox <gi...@apache.org>.
dstandish commented on code in PR #25943:
URL: https://github.com/apache/airflow/pull/25943#discussion_r954072188
##########
airflow/models/taskinstance.py:
##########
@@ -584,8 +584,12 @@ def __init__(
# can be changed when calling 'run'
self.test_mode = False
- self.dataset_event_manager = conf.getimport(
- 'core', 'dataset_event_manager_class', fallback='airflow.datasets.manager.DatasetEventManager'
+ @cached_property
+ def dataset_event_manager(self):
+ return conf.getimport(
+ section='core',
+ key='dataset_event_manager_class',
+ fallback='airflow.datasets.manager.DatasetEventManager',
)()
Review Comment:
i'm actually working on centralizing this logic... it won't just be used on on TIs @uranusjr @blag
--
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
[GitHub] [airflow] uranusjr commented on a diff in pull request #25943: Make dataset_event_manager a cached property on TaskInstance
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #25943:
URL: https://github.com/apache/airflow/pull/25943#discussion_r954042943
##########
airflow/models/taskinstance.py:
##########
@@ -584,8 +584,12 @@ def __init__(
# can be changed when calling 'run'
self.test_mode = False
- self.dataset_event_manager = conf.getimport(
- 'core', 'dataset_event_manager_class', fallback='airflow.datasets.manager.DatasetEventManager'
+ @cached_property
+ def dataset_event_manager(self):
+ return conf.getimport(
+ section='core',
+ key='dataset_event_manager_class',
+ fallback='airflow.datasets.manager.DatasetEventManager',
)()
Review Comment:
Why does this need to be an instance-wide property in the first place? Do we plan to add hook so each task instance can have a different manager?
--
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
[GitHub] [airflow] blag commented on a diff in pull request #25943: Make dataset_event_manager a cached property on TaskInstance
Posted by GitBox <gi...@apache.org>.
blag commented on code in PR #25943:
URL: https://github.com/apache/airflow/pull/25943#discussion_r954035687
##########
airflow/datasets/manager.py:
##########
@@ -31,6 +31,9 @@ class DatasetEventManager(LoggingMixin):
Airflow deployments can use plugins that broadcast dataset events to each other.
"""
+ def __init__(self, **kwargs):
+ super().__init__(**kwargs)
+
Review Comment:
Is this doing anything?
```suggestion
```
--
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