eladkal commented on code in PR #39155: URL: https://github.com/apache/airflow/pull/39155#discussion_r1591446876
########## docs/apache-airflow-providers-tabular/changelog.rst: ########## Review Comment: undo change to this file ########## airflow/providers/tabular/hooks/tabular.py: ########## @@ -14,80 +14,18 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from __future__ import annotations - -from typing import Any - -import requests -from requests import HTTPError - -from airflow.hooks.base import BaseHook - -DEFAULT_TABULAR_URL = "https://api.tabulardata.io/ws/v1"; - -TOKENS_ENDPOINT = "oauth/tokens" - -class TabularHook(BaseHook): - """ - This hook acts as a base hook for tabular services. +"""This module is deprecated. Please use :mod:`airflow.providers.apache.iceberg.hooks.iceberg`.""" - It offers the ability to generate temporary, short-lived - session tokens to use within Airflow submitted jobs. - - :param tabular_conn_id: The :ref:`Tabular connection id<howto/connection:tabular>` - which refers to the information to connect to the Tabular OAuth service. - """ - - conn_name_attr = "tabular_conn_id" - default_conn_name = "tabular_default" - conn_type = "tabular" - hook_name = "Tabular" - - @classmethod - def get_ui_field_behaviour(cls) -> dict[str, Any]: - """Return custom UI field behaviour for Tabular connection.""" - return { - "hidden_fields": ["schema", "port"], - "relabeling": { - "host": "Base URL", - "login": "Client ID", - "password": "Client Secret", - }, - "placeholders": { - "host": DEFAULT_TABULAR_URL, - "login": "client_id (token credentials auth)", - "password": "secret (token credentials auth)", - }, - } - - def __init__(self, tabular_conn_id: str = default_conn_name) -> None: - super().__init__() - self.conn_id = tabular_conn_id - - def test_connection(self) -> tuple[bool, str]: - """Test the Tabular connection.""" - try: - self.get_conn() - return True, "Successfully fetched token from Tabular" - except HTTPError as e: - return False, f"HTTP Error: {e}: {e.response.text}" - except Exception as e: - return False, str(e) - - def get_conn(self) -> str: - """Obtain a short-lived access token via a client_id and client_secret.""" - conn = self.get_connection(self.conn_id) - base_url = conn.host if conn.host else DEFAULT_TABULAR_URL - base_url = base_url.rstrip("/") - client_id = conn.login - client_secret = conn.password - data = {"client_id": client_id, "client_secret": client_secret, "grant_type": "client_credentials"} +from __future__ import annotations - response = requests.post(f"{base_url}/{TOKENS_ENDPOINT}", data=data) - response.raise_for_status() +import warnings - return response.json()["access_token"] +from airflow.exceptions import AirflowProviderDeprecationWarning +from airflow.providers.apache.iceberg.hooks.iceberg import IcebergHook # noqa: F401 Review Comment: In case where user updated tabular provider but did not install the iceberg provider. In such case this import will fail. We need to wrap this with try/catch and raise exception (possibly `AirflowOptionalProviderFeatureException` ?) ########## airflow/providers/tabular/CHANGELOG.rst: ########## @@ -26,6 +26,13 @@ Changelog --------- +1.5.0 +..... Review Comment: ```suggestion ``` No need for the version. Release manager auto pick up the comment at the change log and place it to the chosen release version ########## airflow/providers/tabular/provider.yaml: ########## @@ -25,6 +25,7 @@ state: ready source-date-epoch: 1705912293 # note that those versions are maintained by release manager - do not update them manually versions: + - 1.5.0 Review Comment: ```suggestion ``` ########## docs/apache-airflow-providers-tabular/redirects.txt: ########## @@ -0,0 +1,3 @@ +connections.rst ../apache-airflow-providers-apache-iceberg/connections.rst +changelog.rst ../apache-airflow-providers-apache-iceberg/changelog.rst +index.rst ../apache-airflow-providers-apache-iceberg/index.rst Review Comment: I think we don't need redirect here. It's a different provider -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
