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/06/03 06:45:15 UTC
[GitHub] [airflow] uranusjr opened a new pull request, #24153: Apply per-run log templates to log handlers
uranusjr opened a new pull request, #24153:
URL: https://github.com/apache/airflow/pull/24153
We implemented _recording_ of per-run templates, but didn’t access them in the log handlers to read the logs out. This fixes that. Since the template values are now dynamic, it no longer makes sense to set a static `filename_template` attribute on the handler object (and for ElasticSearch specifically, `log_id_template`), so I deprecated the attributes.
Note that `FileProcessorHandler` does not use the dynamic template, and thus still accepts a static `filename_template`. We could potentially also deprecate that as well, however (and read from configuration instead).
cc @jedcunningham
--
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] potiuk commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1198540254
> @marlock9 @potiuk but how to remove the warning? Is it removing `log_filename_template` from the cfg? If the `log_filename_template` needs to be removed, why is it stll in https://github.com/apache/airflow/blob/main/airflow/config_templates/default_airflow.cfg ?
No. it's about "filename_template" in your config, not about log_flilename_template @uranusjr - am I right?
--
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] potiuk commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1198540962
Wher did you take "log_filename_tamplate" from @joao-aguilera-c ? What led you to thinking this should be removed?
--
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 #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r889416845
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -103,7 +112,10 @@ def __init__(
self.handler: Union[logging.FileHandler, logging.StreamHandler] # type: ignore[assignment]
def _render_log_id(self, ti: TaskInstance, try_number: int) -> str:
- dag_run = ti.get_dagrun()
+ with create_session() as session:
+ dag_run = ti.get_dagrun(session=session)
+ log_id_template = dag_run.get_log_template(session=session).elasticsearch_id
Review Comment:
Ah, right. I think we can simply detect whether there’s a `LogTemplate` model.
--
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 merged pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #24153:
URL: https://github.com/apache/airflow/pull/24153
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1200051260
@marlock9 I agree with your hypothesis, and my custom airflow deployment uses a custom cfg file, and to test it I removed the "log_filename_template" from it.
But unfortunately, the warning continues, and it is so annoying!
--
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 pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1145750181
Looks like tests need some fixes.
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890168119
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
We also need to leave this in case folks have an older provider, right?
##########
tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py:
##########
@@ -65,21 +71,32 @@ def setUp(self):
task_id = 'task_for_testing_cloudwatch_log_handler'
self.dag = DAG(dag_id=dag_id, start_date=date)
task = EmptyOperator(task_id=task_id, dag=self.dag)
- dag_run = DagRun(dag_id=self.dag.dag_id, execution_date=date, run_id="test")
- self.ti = TaskInstance(task=task)
+ dag_run = DagRun(dag_id=self.dag.dag_id, execution_date=date, run_id="test", run_type="scheduled")
+ with create_session() as session:
+ session.add(dag_run)
+ session.commit()
+ session.refresh(dag_run)
+
+ self.ti = TaskInstance(task=task, run_id=dag_run.run_id)
self.ti.dag_run = dag_run
self.ti.try_number = 1
self.ti.state = State.RUNNING
- self.remote_log_stream = f'{dag_id}/{task_id}/{date.isoformat()}/{self.ti.try_number}.log'.replace(
+ self.remote_log_stream = (f'{dag_id}/{task_id}/{date.isoformat()}/{self.ti.try_number}.log').replace(
':', '_'
)
moto.moto_api._internal.models.moto_api_backend.reset()
self.conn = boto3.client('logs', region_name=self.region_name)
- def tearDown(self):
+ yield
+
+ with create_session() as session:
+ session.query(DagRun).delete()
+
Review Comment:
```suggestion
with create_session() as session:
session.query(DagRun).delete()
```
Oops, doing this twice!
##########
airflow/models/dagrun.py:
##########
@@ -1065,11 +1065,11 @@ def schedule_tis(self, schedulable_tis: Iterable[TI], session: Session = NEW_SES
return count
@provide_session
- def get_log_filename_template(self, *, session: Session = NEW_SESSION) -> str:
Review Comment:
We might want to leave a deprecated `get_log_filename_template` around for backcompat? Only there since 2.3.0, but it's still in releases.
--
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] potiuk commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890240351
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
Yeah. Custom logging config + backwards compatibiliyt is a dangerous combo
--
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 pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1217246051
It’s better to keep the arguments there since they still need to support old Airflow versions for now. A better fix would be to remove `filename_template` from `airflow_local_settings` where those handlers are used.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1200063799
@uranusjr
> The config value is now read automatically, so you should remove it from the logging configuration.
Explicitly, which config value should I remove from cfg file?
--
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] github-actions[bot] commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1147701641
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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 pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1200064230
Not the config file, but the [logging config _Python_ file](https://airflow.apache.org/docs/apache-airflow/stable/logging-monitoring/logging-tasks.html#advanced-configuration), or whereever you are modifying logging, e.g. `logging_config_class`.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1201189768
@uranusjr I use the airflow built-in AWS S3 remote logging, my configs are:
```
remote_logging = True
remote_log_conn_id = airflow-aws
remote_base_log_folder = s3://*************/
encrypt_s3_logs = False
```
Maybe there is something outdated on the class that deals with S3 remote logging, right?
--
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 pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1200058180
@marlock9 This is incorrect. The config is still in use and not deprecated. The warning you reference points to _passing the value directly to the log handler_, which is deprecated. The config value is now read automatically, so you should remove it from the logging configuration.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1180482208
After this merge I've been receiving the `DeprecationWarning: Passing filename_template to FileTaskHandler is deprecated and has no effect`. But from the documentation, I'm not sure what I have to do to be in accordance with the 2.3.3 version.
Can anyone help 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1199881897
@uranusjr - maybe you can help ? I am out of ideas.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1199857451
@potiuk In my cfg file, the only two references for 'filename_template" are the "log_filename_template" and "log_processor_filename_template". So I tought they could be related, but I also knew that erasing those configs sounds absurd.
I have no clue why the warning is being presented, as I did not change the cfg, and there is no reference to "filename_template" on it
--
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] marlock9 commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
marlock9 commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1184281034
@potiuk @joao-aguilera-c I think it's because that properties are still in https://github.com/apache/airflow/blob/main/airflow/config_templates/default_airflow.cfg, and for example in our airflow containers we have this file generated on every container run with that props.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1198526770
@marlock9 @potiuk but how to remove the warning? Is it removing `log_filename_template` from the cfg?
If the `log_filename_template` needs to be removed, why is it stll in https://github.com/apache/airflow/blob/main/airflow/config_templates/default_airflow.cfg ?
--
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] Taragolis commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1216834220
@uranusjr Seems like all of this remote task handlers inherited from `FileTaskHandler`
- `airflow.providers.amazon.aws.log.s3_task_handler.S3TaskHandler`
- `airflow.providers.amazon.aws.log.cloudwatch_task_handler.CloudwatchTaskHandler`
- `airflow.providers.google.cloud.log.gcs_task_handler.GCSTaskHandler`
- `airflow.providers.microsoft.azure.log.wasb_task_handler.WasbTaskHandler`
- `airflow.providers.alibaba.cloud.log.oss_task_handler.OSSTaskHandler`
- `airflow.providers.elasticsearch.log.es_task_handler.ElasticsearchTaskHandler`
All of them pass `[logging] log_filename_template` value to class args `filename_template ` but do not use this attribute directly in handler (latest version of providers)
S3 handler
---
https://github.com/apache/airflow/blob/90338398a67cf8d8ffc6ed0418126d2d76002eb9/airflow/config_templates/airflow_local_settings.py#L58-L60
Init
https://github.com/apache/airflow/blob/90338398a67cf8d8ffc6ed0418126d2d76002eb9/airflow/config_templates/airflow_local_settings.py#L193-L201
Pass to `FileTaskHandler`
https://github.com/apache/airflow/blob/90338398a67cf8d8ffc6ed0418126d2d76002eb9/airflow/providers/amazon/aws/log/s3_task_handler.py#L28-L36
Remove this parameter from initialise this handler remove this warning, however I do not check all previous version of providers and is it possible that previously it uses.
--
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] joao-aguilera-c commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
joao-aguilera-c commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1208519773
@uranusjr what I find strange is that I'm using the 2.3.3 version of airflow, not customized in any way (excepting on the cfg). Here is a example of where the warning appears:
`airflow db init`
> It’s more likely something that uses the class is doing it wrong.
It seems that airflow is this 'something'
--
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] potiuk commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1183500873
Review your logging configuration, find wher filename_templete is passed to a class deriviing from FileTaskHandler and remove it. You can find more info where to look in airlfow's "advanced log configuration" - just read it carefully and find where your logs are configured.
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r889213246
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -103,7 +112,10 @@ def __init__(
self.handler: Union[logging.FileHandler, logging.StreamHandler] # type: ignore[assignment]
def _render_log_id(self, ti: TaskInstance, try_number: int) -> str:
- dag_run = ti.get_dagrun()
+ with create_session() as session:
+ dag_run = ti.get_dagrun(session=session)
+ log_id_template = dag_run.get_log_template(session=session).elasticsearch_id
Review Comment:
This needs more work to make it backwards compatible, or we bump the min Airflow version.
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890230732
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
Looking again, this ships with core, so this is safe (I mistakenly thought this was for ES). However, if folks have their own logging config, it'll blow up. We need to keep `filename_template` in `FileTaskHandler` and just warn about it.
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890231354
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
Which we already do, so I think we are good here.
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890157130
##########
tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py:
##########
@@ -65,21 +71,32 @@ def setUp(self):
task_id = 'task_for_testing_cloudwatch_log_handler'
self.dag = DAG(dag_id=dag_id, start_date=date)
task = EmptyOperator(task_id=task_id, dag=self.dag)
- dag_run = DagRun(dag_id=self.dag.dag_id, execution_date=date, run_id="test")
- self.ti = TaskInstance(task=task)
+ dag_run = DagRun(dag_id=self.dag.dag_id, execution_date=date, run_id="test", run_type="scheduled")
+ with create_session() as session:
+ session.add(dag_run)
+ session.commit()
+ session.refresh(dag_run)
+
+ self.ti = TaskInstance(task=task, run_id=dag_run.run_id)
self.ti.dag_run = dag_run
self.ti.try_number = 1
self.ti.state = State.RUNNING
- self.remote_log_stream = f'{dag_id}/{task_id}/{date.isoformat()}/{self.ti.try_number}.log'.replace(
+ self.remote_log_stream = (f'{dag_id}/{task_id}/{date.isoformat()}/{self.ti.try_number}.log').replace(
':', '_'
)
moto.moto_api._internal.models.moto_api_backend.reset()
self.conn = boto3.client('logs', region_name=self.region_name)
- def tearDown(self):
+ yield
+
+ with create_session() as session:
+ session.query(DagRun).delete()
+
Review Comment:
```suggestion
```
Oops, doing this twice!
--
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 #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r889641002
##########
airflow/providers/elasticsearch/log/es_task_handler.py:
##########
@@ -103,7 +112,10 @@ def __init__(
self.handler: Union[logging.FileHandler, logging.StreamHandler] # type: ignore[assignment]
def _render_log_id(self, ti: TaskInstance, try_number: int) -> str:
- dag_run = ti.get_dagrun()
+ with create_session() as session:
+ dag_run = ti.get_dagrun(session=session)
+ log_id_template = dag_run.get_log_template(session=session).elasticsearch_id
Review Comment:
Added a compat layer.
--
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] Taragolis commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1217739505
> It’s better to keep the arguments there since they still need to support old Airflow versions for now. A better fix would be to remove filename_template from airflow_local_settings where those handlers are used.
Yeah, I also think better to set `filename_template` to None in `airflow_local_settings` so it won't affect old version of providers which have `filename_template` as mandatory arg.
I would create a PR
--
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 pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1202153532
It shouldn’t be in the class—the warning is emitted when logging is _instantiated_. It’s more likely something that _uses_ the class is doing it wrong.
--
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] marlock9 commented on pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
marlock9 commented on PR #24153:
URL: https://github.com/apache/airflow/pull/24153#issuecomment-1199966788
@potiuk IMHO, the main problem is that this exact pull request have deleted using of `log_filename_template` and now it shows warning about deprecation of using it: https://github.com/apache/airflow/pull/24153/files#diff-e7f34f73940eb52d92bb991abedc1c963431c5373c12dff739c8fb7d03e93d3aR53
But this property was not removed from `airflow/config_templates/default_airflow.cfg` from which the default config generates on first run of airflow, it was removed only from `airflow/config_templates/airflow_local_settings.py` and `airflow/config_templates/default_test.cfg`.
So now, even when having this parameter deleted from environment variables this parameter is passed to airflow from that generated config. Setting value to empty string in env vars is also not helpful, because the check is for None type.
--
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] ashb commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890227045
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
It does. Dang.
```
Unable to load the config, contains a configuration error.
Traceback (most recent call last):
File "/usr/lib/python3.10/logging/config.py", line 565, in configure
handler = self.configure_handler(handlers[name])
File "/usr/lib/python3.10/logging/config.py", line 746, in configure_handler
result = factory(**kwargs)
TypeError: FileTaskHandler.__init__() got an unexpected keyword argument 'flibble'
```
--
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 commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890373350
##########
airflow/models/dagrun.py:
##########
@@ -1065,11 +1065,11 @@ def schedule_tis(self, schedulable_tis: Iterable[TI], session: Session = NEW_SES
return count
@provide_session
- def get_log_filename_template(self, *, session: Session = NEW_SESSION) -> str:
Review Comment:
I added it back.
--
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] ashb commented on a diff in pull request #24153: Apply per-run log templates to log handlers
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #24153:
URL: https://github.com/apache/airflow/pull/24153#discussion_r890226318
##########
airflow/config_templates/airflow_local_settings.py:
##########
@@ -82,7 +82,6 @@
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
- 'filename_template': FILENAME_TEMPLATE,
Review Comment:
Probably, yes. Unless -- unless having extra parameters causes a problem for logging framework?
--
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