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 deprecated parts from Slack provider #33557

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Aug 20, 2023

Most of the deprecations are made for about 1 year ago, I think it is a good time to remove it

In additional make slack hooks constructors are keyword arguments only. It would make further changes and deprecations more easier

ToDo:

  • Update documentation if required
  • Add breaking changes

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis
Copy link
Contributor Author

Ahhhh.... There is also exists deprecated operator SnowflakeToSlackOperator.
Seems like also required remove deprecations for snowflake provider first

@Taragolis Taragolis force-pushed the slack-remove-remove-deprecated-stuff branch from 34b3fcd to c91df3a Compare August 21, 2023 18:24
@Taragolis Taragolis force-pushed the slack-remove-remove-deprecated-stuff branch from c91df3a to df69d1f Compare August 21, 2023 18:27
@Taragolis Taragolis marked this pull request as ready for review August 21, 2023 18:28
@@ -27,6 +27,44 @@
Changelog
---------

8.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's a breaking change we need also to add 8.0.0 to the provider yaml

@potiuk potiuk merged commit ed6a4fd into apache:main Aug 24, 2023
42 checks passed
@Taragolis Taragolis deleted the slack-remove-remove-deprecated-stuff branch August 31, 2023 09:10
token: str | None = None,
slack_conn_id: str | None = None,
*,
slack_conn_id: str,
Copy link
Contributor

Choose a reason for hiding this comment

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

default_conn_name is not really used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I proposed a fix for it in this PR: #34548

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better say feature. Without removal in this PR it was quite difficult to do that.
Let's move discussion in to your PR

Copy link
Contributor

@Khrol Khrol Sep 22, 2023

Choose a reason for hiding this comment

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

Yes, it's more an improvement of course. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants