Taragolis commented on code in PR #36953: URL: https://github.com/apache/airflow/pull/36953#discussion_r1477381779
########## airflow/providers/teradata/hooks/teradata.py: ########## @@ -0,0 +1,212 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""A Airflow Hook for interacting with Teradata SQL Server.""" +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, TypeVar + +import sqlalchemy +import teradatasql +from teradatasql import TeradataConnection + +from airflow.providers.common.sql.hooks.sql import DbApiHook + +T = TypeVar("T") Review Comment: This TypeVar doesn't use anywhere ########## tests/providers/teradata/operators/test_teradata.py: ########## @@ -0,0 +1,122 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from unittest import mock +from unittest.mock import MagicMock, Mock + +from airflow.models.dag import DAG +from airflow.providers.common.sql.hooks.sql import fetch_all_handler +from airflow.utils import timezone + +from airflow.providers.teradata.hooks.teradata import TeradataHook +from airflow.providers.teradata.operators.teradata import TeradataOperator + + +from airflow.exceptions import AirflowException + +DEFAULT_DATE = timezone.datetime(2015, 1, 1) +TEST_DAG_ID = "unit_test_dag" + + +class TestTeradataOperator: + def setup_method(self): + args = {"owner": "airflow", "start_date": DEFAULT_DATE} + dag = DAG(TEST_DAG_ID, default_args=args) + self.dag = dag Review Comment: Seems like this part not uses anywhere in to the tests ########## airflow/providers/teradata/hooks/teradata.py: ########## @@ -0,0 +1,212 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""A Airflow Hook for interacting with Teradata SQL Server.""" +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, TypeVar + +import sqlalchemy +import teradatasql +from teradatasql import TeradataConnection + +from airflow.providers.common.sql.hooks.sql import DbApiHook + +T = TypeVar("T") +if TYPE_CHECKING: + from airflow.models.connection import Connection + + +class TeradataHook(DbApiHook): + """General hook for interacting with Teradata SQL Database. + + This module contains basic APIs to connect to and interact with Teradata SQL Database. It uses teradatasql + client internally as a database driver for connecting to Teradata database. The config parameters like + Teradata DB Server URL, username, password and database name are fetched from the predefined connection + config connection_id. It raises an airflow error if the given connection id doesn't exist. + + See :doc:` docs/apache-airflow-providers-teradata/connections/teradata.rst` for full documentation. Review Comment: This won't render in API documentation. You could check documentation locally by run: ```console breeze build-docs teradata ``` Instead of reference to the rst files, better use targets, because it do not breaks if page moved into the future. Example --- **Define target** https://github.com/apache/airflow/blob/faa32f23e824ec8dd00b296ce9d8bd239ac0437f/docs/apache-airflow-providers-slack/connections/slack.rst?plain=1#L20-L23 You've already defined it into the `docs/apache-airflow-providers-teradata/connections/teradata.rst` **Use it into the docstrings** https://github.com/apache/airflow/blob/46470aba68e5ebeee24a03dc22d012a50ee287ad/airflow/providers/slack/hooks/slack.py#L67 https://github.com/apache/airflow/blob/46470aba68e5ebeee24a03dc22d012a50ee287ad/airflow/providers/slack/hooks/slack.py#L92-L93 ########## airflow/providers/teradata/operators/teradata.py: ########## @@ -0,0 +1,71 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# 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 Sequence + +from airflow.providers.common.sql.operators.sql import SQLExecuteQueryOperator +from airflow.providers.teradata.hooks.teradata import TeradataHook + + +class TeradataOperator(SQLExecuteQueryOperator): + """ + General Teradata Operator to execute queries on Teradata Database. + + Executes sql statements in the Teradata SQL Database using teradatasql jdbc driver + .. seealso:: + For more information on how to use this operator, take a look at the guide: + :ref:`howto/operator:TeradataOperator` Review Comment: Missing new line before RST block ```suggestion .. seealso:: For more information on how to use this operator, take a look at the guide: :ref:`howto/operator:TeradataOperator` ``` ########## docs/apache-airflow-providers-teradata/redirects.txt: ########## Review Comment: This redirection not required. -- 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]
