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

Remove depreciation warning when use default remote tasks logging handlers #25764

Merged

Conversation

Taragolis
Copy link
Contributor

All of the default remote task handlers which inherited from FileTaskHandler pass value from [logging] log_filename_template value to class args filename_template.

  • 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

As result when enabled remote logging (e.g. S3 Logging) quite a few deprecation warnings appear in airflow services logs

/home/airflow/.local/lib/python3.9/site-packages/airflow/utils/log/file_task_handler.py:52 DeprecationWarning: Passing filename_template to FileTaskHandler is deprecated and has no effect

This handlers do not use internally filename_template attribute ( check for main and first version of each handlers), so it should be safe set None for all of them.

closes: #25743
related: #24153

cc: @uranusjr

# FILENAME_TEMPLATE only uses in Remote Logging Handlers since Airflow 2.3.3
# All of these handlers inherited from FileTaskHandler and providing any value rather than None
# would raise deprecation warning.
FILENAME_TEMPLATE: Optional[str] = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find any usage of this variable outside of airflow/config_templates/airflow_local_settings.py

Copy link
Member

Choose a reason for hiding this comment

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

It shouldn’t be used anywhere else (should just read from conf). This can be removed.

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

I think we can remove FILENAME_TEMPLATE outright, but this is OK as well.

@Taragolis Taragolis force-pushed the fix-remote-logging-depreciation-warning branch from f64af17 to c50c151 Compare August 18, 2022 08:21
@ephraimbuddy ephraimbuddy added this to the Airflow 2.3.4 milestone Aug 19, 2022
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Aug 19, 2022
@ephraimbuddy ephraimbuddy merged commit da616a1 into apache:main Aug 19, 2022
ephraimbuddy pushed a commit that referenced this pull request Aug 19, 2022
@Taragolis Taragolis deleted the fix-remote-logging-depreciation-warning branch January 14, 2023 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

DeprecationWarning: Passing filename_template to FileTaskHandler is deprecated and has no effect
3 participants