utkarsharma2 commented on code in PR #34921:
URL: https://github.com/apache/airflow/pull/34921#discussion_r1378742902


##########
airflow/providers/cohere/hooks/cohere.py:
##########
@@ -0,0 +1,82 @@
+#
+# 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 functools import cached_property
+from typing import Any
+
+import cohere
+
+from airflow.hooks.base import BaseHook
+
+
+class CohereHook(BaseHook):
+    """
+    Use Cohere Python SDK to interact with Cohere platform.
+
+    .. seealso:: https://docs.cohere.com/docs
+
+    :param conn_id: :ref:`Cohere connection id <howto/connection:cohere>`
+    """
+
+    conn_name_attr = "conn_id"
+    default_conn_name = "cohere_default"
+    conn_type = "cohere"
+    hook_name = "Cohere"
+
+    def __init__(
+        self,
+        conn_id: str = default_conn_name,
+        timeout: int | None = None,
+        max_retries: int | None = None,

Review Comment:
   Thanks, I have added the missing string.  



##########
airflow/providers/cohere/hooks/cohere.py:
##########
@@ -0,0 +1,82 @@
+#
+# 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 functools import cached_property
+from typing import Any
+
+import cohere
+
+from airflow.hooks.base import BaseHook
+
+
+class CohereHook(BaseHook):
+    """
+    Use Cohere Python SDK to interact with Cohere platform.
+
+    .. seealso:: https://docs.cohere.com/docs
+
+    :param conn_id: :ref:`Cohere connection id <howto/connection:cohere>`
+    """
+
+    conn_name_attr = "conn_id"
+    default_conn_name = "cohere_default"
+    conn_type = "cohere"
+    hook_name = "Cohere"
+
+    def __init__(
+        self,
+        conn_id: str = default_conn_name,
+        timeout: int | None = None,
+        max_retries: int | None = None,
+    ) -> None:
+        super().__init__()
+        self.conn_id = conn_id
+        self.client = None

Review Comment:
   Not needed as such, but I guess creating a variable outside `__init__` isn't 
ideal. 



##########
tests/system/providers/cohere/example_cohere_embedding_operator.py:
##########
@@ -0,0 +1,49 @@
+# 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 datetime import datetime
+
+from airflow import DAG
+from airflow.decorators import task
+from airflow.providers.cohere.operators.embedding import 
CohereEmbeddingOperator
+
+with DAG("example_cohere_embedding", schedule=None, start_date=datetime(2023, 
1, 1), catchup=False) as dag:

Review Comment:
   @josh-fell We later require it for below code. Referenced it from existing 
code - 
https://github.com/apache/airflow/blob/3724a029dadf4678d1cc89049b247f6d2bc233e2/tests/system/providers/slack/example_sql_to_slack.py#L56
 But I'm happy to change if there is workaround.
   ```
   from tests.system.utils import get_test_run
   
   # Needed to run the example DAG with pytest (see: 
tests/system/README.md#run_via_pytest)
   test_run = get_test_run(dag)
   ```



##########
airflow/providers/cohere/operators/embedding.py:
##########
@@ -0,0 +1,72 @@
+# 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 functools import cached_property
+from typing import TYPE_CHECKING, Any, Sequence
+
+from airflow.models import BaseOperator
+from airflow.providers.cohere.hooks.cohere import CohereHook
+
+if TYPE_CHECKING:
+    from airflow.utils.context import Context
+
+
+class CohereEmbeddingOperator(BaseOperator):
+    """Creates the embedding base by interacting with cohere hosted services.
+
+    .. seealso::
+        For more information on how to use this operator, take a look at the 
guide:
+        :ref:`howto/operator:CohereEmbeddingOperator`
+
+    :param conn_id: Optional. The name of the Airflow connection to get 
connection
+        information for Cohere. Defaults to "cohere_default".
+    :param input_text: list of text items that need to be embedded. Only one 
of input_text or input_callable
+        should be provided.
+    :param input_callable: The callable that provides the input texts to 
generate embeddings for.
+        Only one of input_text or input_callable should be provided.
+    :param input_callable_args: The list of arguments to be passed to 
``input_callable``.
+    :param input_callable_kwargs: The kwargs to be passed to 
``input_callable``.

Review Comment:
   Ya, we decided not to do it. Here is the discussion - 
https://github.com/apache/airflow/pull/34921#discussion_r1358525838 
   
   But I missed removing the Docstring. Thanks for pointing that out. 



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

Reply via email to