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

log handler deprecated filename_template argument removal #41552

Merged

Conversation

dirrao
Copy link
Contributor

@dirrao dirrao commented Aug 17, 2024

Passing filename_template to a log handler is deprecated and has no effect. So, removing deprecated filename_template argument in log handler.

@dirrao dirrao self-assigned this Aug 17, 2024
@dirrao dirrao added the airflow3.0:candidate Potential candidates for Airflow 3.0 label Aug 17, 2024
@dirrao dirrao requested review from uranusjr and potiuk August 17, 2024 11:05
@uranusjr uranusjr added the airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes label Aug 17, 2024
@dirrao dirrao requested a review from romsharon98 August 19, 2024 06:43
@uranusjr uranusjr merged commit 716c430 into apache:main Aug 20, 2024
80 checks passed
@eladkal
Copy link
Contributor

eladkal commented Aug 20, 2024

This PR involves both core and provider changes.
Please clarify what it means for new versions of providers (that will include this PR) but installed with Airflow 2.
Something is odd here.

The parameter in question is public API which is why we deprecate it first. Now main is Airflow 3 so we can remove it but providers must still be compatible with Airflow 2... so how can we remove it from providers safely now?

@dirrao
Copy link
Contributor Author

dirrao commented Aug 20, 2024

This PR involves both core and provider changes. Please clarify what it means for new versions of providers (that will include this PR) but installed with Airflow 2. Something is odd here.

Right now, this field is not being used and has no effect. The filename_template is optional argument with default value None.
We can still able to install and use latest providers on airflow 2 without any issue.

The parameter in question is public API which is why we deprecate it first. Now main is Airflow 3 so we can remove it but providers must still be compatible with Airflow 2... so how can we remove it from providers safely now?

As I said in the above statement, the providers can still work with airflow 2.

@potiuk
Copy link
Member

potiuk commented Aug 20, 2024

@eladkal is right - if someone installs the new provider witn an old configuration in Airlfow 2 with filename_template, it will just fail with "attemptign to iniitalize handlew with "filename_template" argument passed but it's missing" (or smth like that).

@dirrao
Copy link
Contributor Author

dirrao commented Aug 20, 2024

I see the keyword arguments for the all the

@eladkal is right - if someone installs the new provider witn an old configuration in Airlfow 2 with filename_template, it will just fail with "attemptign to iniitalize handlew with "filename_template" argument passed but it's missing" (or smth like that).

Yah. You are right. Most of them are supporting keyword arguments except one/two (like. es handler).

@dirrao
Copy link
Contributor Author

dirrao commented Aug 20, 2024

@eladkal / @potiuk
Let me know if you want me to revert it for providers alone.

@potiuk
Copy link
Member

potiuk commented Aug 20, 2024

Just reverting will not be enough, but I think if the base Handler will accept extra arguments (and ignores them) we should be home

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
airflow3.0:breaking Candidates for Airflow 3.0 that contain breaking changes airflow3.0:candidate Potential candidates for Airflow 3.0 area:logging area:providers provider:alibaba provider:amazon-aws AWS/Amazon - related issues provider:apache-hdfs provider:elasticsearch provider:google Google (including GCP) related issues provider:microsoft-azure Azure-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants