[GitHub] [airflow] eladkal commented on a diff in pull request #25350: Move all "old" SQL operators to common.sql providers

[GitBox <gi...@apache.org>]

eladkal commented on code in PR #25350:
URL: https://github.com/apache/airflow/pull/25350#discussion_r932919648


##########
airflow/operators/sql.py:
##########
@@ -15,543 +15,23 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, SupportsAbs, Union
-
-from airflow.compat.functools import cached_property
-from airflow.exceptions import AirflowException
-from airflow.hooks.base import BaseHook
-from airflow.models import BaseOperator, SkipMixin
-from airflow.providers.common.sql.hooks.sql import DbApiHook
-from airflow.utils.context import Context
-
-
-def parse_boolean(val: str) -> Union[str, bool]:
-    """Try to parse a string into boolean.
-
-    Raises ValueError if the input is not a valid true- or false-like string value.
-    """
-    val = val.lower()
-    if val in ('y', 'yes', 't', 'true', 'on', '1'):
-        return True
-    if val in ('n', 'no', 'f', 'false', 'off', '0'):
-        return False
-    raise ValueError(f"{val!r} is not a boolean-like string value")
-
-
-class BaseSQLOperator(BaseOperator):
-    """
-    This is a base class for generic SQL Operator to get a DB Hook
-
-    The provided method is .get_db_hook(). The default behavior will try to
-    retrieve the DB hook based on connection type.
-    You can custom the behavior by overriding the .get_db_hook() method.
-    """
-
-    def __init__(
-        self,
-        *,
-        conn_id: Optional[str] = None,
-        database: Optional[str] = None,
-        hook_params: Optional[Dict] = None,
-        **kwargs,
-    ):
-        super().__init__(**kwargs)
-        self.conn_id = conn_id
-        self.database = database
-        self.hook_params = {} if hook_params is None else hook_params
-
-    @cached_property
-    def _hook(self):
-        """Get DB Hook based on connection type"""
-        self.log.debug("Get connection for %s", self.conn_id)
-        conn = BaseHook.get_connection(self.conn_id)
-
-        hook = conn.get_hook(hook_params=self.hook_params)
-        if not isinstance(hook, DbApiHook):
-            raise AirflowException(
-                f'The connection type is not supported by {self.__class__.__name__}. '
-                f'The associated hook should be a subclass of `DbApiHook`. Got {hook.__class__.__name__}'
-            )
-
-        if self.database:
-            hook.schema = self.database
-
-        return hook
-
-    def get_db_hook(self) -> DbApiHook:
-        """
-        Get the database hook for the connection.
-
-        :return: the database hook object.
-        :rtype: DbApiHook
-        """
-        return self._hook
-
-
-class SQLCheckOperator(BaseSQLOperator):

Review Comment:
   Removing the operators is a breaking change....
   We need to deprecate them



##########
airflow/providers/common/sql/CHANGELOG.rst:
##########
@@ -24,6 +24,11 @@
 Changelog
 ---------
 
+1.1.0

Review Comment:
   Did you mean to add it without the log entry of the change?



-- 
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