You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "rawwar (via GitHub)" <gi...@apache.org> on 2024/02/10 15:25:43 UTC
[PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
rawwar opened a new pull request, #37307:
URL: https://github.com/apache/airflow/pull/37307
closes: #37158
Adds support for pinecone-client ~= 3.0.x
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567582624
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
Review Comment:
Added them
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567554452
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
Review Comment:
> check all these attribute in __init__ all at once? (e.g., all([...]))`
@Lee-W , I'm sorry. I need help understanding this. Do you want to check if the `environment` and `region` are passed directly in the arguments? I am unsure how we can use `all` in `__init__` here as these properties can be part of the connection or the object and might be missing.
For instance, a user might not even pass `region` when using a pod-based index and
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1570128288
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,132 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ """
+ Get a PodSpec object.
+
+ :param replicas: The number of replicas.
+ :param shards: The number of shards.
+ :param pods: The number of pods.
+ :param pod_type: The type of pod.
+ :param metadata_config: The metadata configuration.
+ :param source_collection: The source collection.
+ :param environment: The environment to use when creating the index.
+ """
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
Review Comment:
```suggestion
def get_serverless_spec_obj(self, cloud: str, region: str | None = None) -> ServerlessSpec:
```
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,132 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
Review Comment:
nitpick: does the order of these arguments matter? if not, should we make them keyword arguments
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,132 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ """
+ Get a PodSpec object.
+
+ :param replicas: The number of replicas.
+ :param shards: The number of shards.
+ :param pods: The number of pods.
+ :param pod_type: The type of pod.
+ :param metadata_config: The metadata configuration.
+ :param source_collection: The source collection.
+ :param environment: The environment to use when creating the index.
+ """
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ """
+ Get a ServerlessSpec object.
+
+ :param cloud: The cloud provider.
+ :param region: The region to use when creating the index.
+ """
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
index_name: str,
dimension: int,
- index_type: str | None = "approximated",
+ spec: ServerlessSpec | PodSpec,
metric: str | None = "cosine",
- replicas: int | None = 1,
- shards: int | None = 1,
- pods: int | None = 1,
- pod_type: str | None = "p1",
- index_config: dict[str, str] | None = None,
- metadata_config: dict[str, str] | None = None,
- source_collection: str | None = "",
timeout: int | None = None,
) -> None:
"""
Create a new index.
- .. seealso:: https://docs.pinecone.io/reference/create_index/
-
- :param index_name: The name of the index to create.
- :param dimension: the dimension of vectors that would be inserted in the index
- :param index_type: type of index, one of {"approximated", "exact"}, defaults to "approximated".
- :param metric: type of metric used in the vector index, one of {"cosine", "dotproduct", "euclidean"}
- :param replicas: the number of replicas, defaults to 1.
- :param shards: the number of shards per index, defaults to 1.
- :param pods: Total number of pods to be used by the index. pods = shard*replicas
- :param pod_type: the pod type to be used for the index. can be one of p1 or s1.
- :param index_config: Advanced configuration options for the index
- :param metadata_config: Configuration related to the metadata index
- :param source_collection: Collection name to create the index from
- :param timeout: Timeout for wait until index gets ready.
+ :param index_name: The name of the index.
+ :param dimension: The dimension of the vectors to be indexed.
+ :param spec: Pass a `ServerlessSpec` object to create a serverless index or a `PodSpec` object to create a pod index.
+ :param metric: The metric to use.
+ :param timeout: The timeout to use.
"""
- pinecone.create_index(
+ self.pc.create_index(
name=index_name,
- timeout=timeout,
- index_type=index_type,
dimension=dimension,
+ spec=spec,
metric=metric,
- pods=pods,
- replicas=replicas,
- shards=shards,
- pod_type=pod_type,
- metadata_config=metadata_config,
- source_collection=source_collection,
- index_config=index_config,
+ timeout=timeout,
)
- @staticmethod
- def describe_index(index_name: str) -> Any:
+ def describe_index(self, index_name: str) -> Any:
"""
Retrieve information about a specific index.
:param index_name: The name of the index to describe.
"""
- return pinecone.describe_index(name=index_name)
+ return self.pc.describe_index(name=index_name)
- @staticmethod
- def delete_index(index_name: str, timeout: int | None = None) -> None:
+ def delete_index(self, index_name: str, timeout: int | None = None) -> None:
"""
Delete a specific index.
:param index_name: the name of the index.
:param timeout: Timeout for wait until index gets ready.
"""
- pinecone.delete_index(name=index_name, timeout=timeout)
+ self.pc.delete_index(name=index_name, timeout=timeout)
- @staticmethod
- def configure_index(index_name: str, replicas: int | None = None, pod_type: str | None = "") -> None:
+ def configure_index(
+ self, index_name: str, replicas: int | None = None, pod_type: str | None = ""
+ ) -> None:
"""
Change the current configuration of the index.
:param index_name: The name of the index to configure.
:param replicas: The new number of replicas.
:param pod_type: the new pod_type for the index.
"""
- pinecone.configure_index(name=index_name, replicas=replicas, pod_type=pod_type)
+ self.pc.configure_index(name=index_name, replicas=replicas, pod_type=pod_type)
@staticmethod
Review Comment:
```suggestion
```
I guess we probably should remove it as well?
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488793501
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
If pinecone maintainers follow semver the minor/feature versions shouldn't introduce breaking changes. I can't find any information that pinecone have a plan to release breaking changes in 3.1.
Usual we do not use upper bound limits, only in edge cases: e.g. it is known fact that new major (rarely minor) of SDK will introduce breaking changes.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567554452
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
Review Comment:
> check all these attribute in __init__ all at once? (e.g., all([...]))`
@Lee-W , I'm sorry. I need help understanding this. Do you want to check if the `environment` and `region` are passed directly in the arguments? I am unsure how we can use `all` in `__init__` here as these properties can be part of the connection or the object and might be missing.
For instance, a user might not even pass `region` when using a pod-based index and
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1581639150
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,132 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
Review Comment:
updated it. Thanks
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,132 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
Review Comment:
updated it. Thanks
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1485549912
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
This would not work as expected, e.g. maximum allowed version would be 3.1 - 0.0.1
You could check it by use [`packaging`](https://packaging.pypa.io/en/stable/index.html) library
```python
from packaging.requirements import SpecifierSet
from packaging.version import Version
specifiers = [
SpecifierSet("~=3.0.0"),
SpecifierSet("~=3.0"),
SpecifierSet(">=3,<4"),
]
versions = [
Version("2.2.4"),
Version("3.0.0"),
Version("3.0.1"),
Version("3.1.0"),
Version("4.0.0"),
]
for spec in specifiers:
print(f" {spec} ".center(72, "="))
for version in versions:
print(f"Version {version} in spec? {version in spec}")
```
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1563914685
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,29 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. note::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
+
+* PineconeHook's ``create_index`` method now requies to pass a Specification object for creation of Pod or Serverless based index.
+
+Features
+~~~~~~~~
+
+* supports pod based index creation using ``CreatePodIndexOperator`` Operator
+* Supports Serverless index creation using ``CreateServerlessIndexOperator`` Operator
+* Supports PINECONE_DEBUG_CURL environment variable via Connection to enable curl debugging
+
+Misc
+~~~~
+* Support ``pinecone-client`` version >=3.0.0
+
Review Comment:
```suggestion
```
This one not required, all other things except breaking changes will added during release process
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,29 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. note::
Review Comment:
A bit more severity rather than just note
```suggestion
.. warning::
```
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,29 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. note::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
Review Comment:
Could you also add what is changed and what actions/steps required to do by end users to migrate to new version
Some examples:
- https://github.com/apache/airflow/blob/main/airflow/providers/apache/hive/CHANGELOG.rst#breaking-changes
- https://github.com/apache/airflow/blob/main/airflow/providers/amazon/CHANGELOG.rst#800
- https://github.com/apache/airflow/blob/main/airflow/providers/slack/CHANGELOG.rst#800
- https://github.com/apache/airflow/blob/main/airflow/providers/google/CHANGELOG.rst#breaking-changes
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1563936620
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,29 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. note::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
Review Comment:
Thank you. I'll go through them and update
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1566550167
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,80 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self,
+ conn_id: str = default_conn_name,
+ environment: str | None = None,
+ region: str | None = None,
+ api_key: str | None = None,
Review Comment:
@sunank200 , I have removed `api_key` as an arg
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W merged PR #37307:
URL: https://github.com/apache/airflow/pull/37307
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on PR #37307:
URL: https://github.com/apache/airflow/pull/37307#issuecomment-2075351057
@sunank200 , @Lee-W, Do you have any more feedback for the PR?
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1580860624
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
I've changed it to `pinecone_client`
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1496742109
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,118 +132,98 @@ def upsert(
)
@staticmethod
+ def get_pod_spec_obj(environment, replicas, shards, pods, pod_type, metadata_config, source_collection):
+ return PodSpec(
+ environment=environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ @staticmethod
+ def get_serverless_spec_obj(cloud, region):
+ return ServerlessSpec(cloud=cloud, region=region)
+
def create_index(
+ self,
index_name: str,
dimension: int,
- index_type: str | None = "approximated",
+ spec: ServerlessSpec | PodSpec,
metric: str | None = "cosine",
- replicas: int | None = 1,
- shards: int | None = 1,
- pods: int | None = 1,
- pod_type: str | None = "p1",
- index_config: dict[str, str] | None = None,
- metadata_config: dict[str, str] | None = None,
- source_collection: str | None = "",
Review Comment:
Yes, I will include them in the changelog. will request for a feedback once I am done.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1579128144
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,36 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. warning::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
+
+* ``log_level`` field is removed from the Connections as it is not used by the provider anymore.
+* ``PineconeHook.get_conn`` now returns ``Connection`` object instead of ``PineconeConnection`` object. Use ``pc`` property to access the Pinecone client.
+* Following ``PineconeHook`` methods are converted from static methods to instance methods. Hence, Initialization is required to use these now:
Review Comment:
Why this this change needed? Can't it be implemented in a way that it is still static?
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
Can this be called hook?
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
Can you document all the parameters supported as part of specs or provide the link to pinecone docs for the same in the doc-string?
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488772082
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
Regarding the pin, I did want to limit >=3.0.0 and <3.1.0. Since, <2 wouldn't work for us and 3.1 can possibly have breaking changes. Once Pinecone releases 3.1, I thought we can test them out and then change pin to `pinecone-client>3,<3.2` . Do you think its a reasonable approach?
And thanks for introducing me to `SpecifierSet`. That is new to me.
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488788371
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -63,43 +64,40 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
Review Comment:
So, that can be something they can pass as an argument to the hook or the operator. This way, they can actually create indexes in different regions without restrictions.
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on PR #37307:
URL: https://github.com/apache/airflow/pull/37307#issuecomment-2054005270
I am not sure why a few tests are failing. But, I'm looking into it.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1580751492
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
+ pinecone_host = self.conn.host
+ extras = self.conn.extra_dejson
pinecone_project_id = extras.get("project_id")
- log_level = extras.get("log_level", None)
- pinecone.init(
- api_key=api_key,
- environment=pinecone_environment,
- host=pinecone_host,
- project_name=pinecone_project_id,
- log_level=log_level,
- )
+ enable_curl_debug = extras.get("debug_curl")
+ if enable_curl_debug:
+ os.environ["PINECONE_DEBUG_CURL"] = "true"
+ return Pinecone(api_key=self.api_key, host=pinecone_host, project_id=pinecone_project_id)
+
+ def get_conn(self) -> Connection:
Review Comment:
Should we try something like
```python
@property
def conn(self) -> Connection:
```
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
I don't think it's actaully a hook. What about `pc_client` or maybe `pinecone_client` is even better?
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1579077234
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -45,7 +45,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
Review Comment:
Thanks. I will remove it
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1581266062
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
@rawwar
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1580864264
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,36 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. warning::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
+
+* ``log_level`` field is removed from the Connections as it is not used by the provider anymore.
+* ``PineconeHook.get_conn`` now returns ``Connection`` object instead of ``PineconeConnection`` object. Use ``pc`` property to access the Pinecone client.
+* Following ``PineconeHook`` methods are converted from static methods to instance methods. Hence, Initialization is required to use these now:
Review Comment:
Earlier, imported pinecone module object can be used. Hence, staticmethod made sense. But now, we need to initialize a pinecone object. Hence, having them static methods and then again sending the connection object to all of them didn't make sense to me. hence, this change
Earlier pinecone connection creation:
```
import pinecone
pinecone.init(
api_key='XXX',
environment='us-east1-gcp'
)
```
[This](https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b) migration guide talks about this in detail
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1485546051
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -63,43 +64,40 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
Review Comment:
I think we need to keep this one, because it should use in some places, e.g. in Creating a pod index:
https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "pankajastro (via GitHub)" <gi...@apache.org>.
pankajastro commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1493785686
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,118 +132,98 @@ def upsert(
)
@staticmethod
+ def get_pod_spec_obj(environment, replicas, shards, pods, pod_type, metadata_config, source_collection):
+ return PodSpec(
+ environment=environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ @staticmethod
+ def get_serverless_spec_obj(cloud, region):
+ return ServerlessSpec(cloud=cloud, region=region)
+
def create_index(
+ self,
index_name: str,
dimension: int,
- index_type: str | None = "approximated",
+ spec: ServerlessSpec | PodSpec,
metric: str | None = "cosine",
- replicas: int | None = 1,
- shards: int | None = 1,
- pods: int | None = 1,
- pod_type: str | None = "p1",
- index_config: dict[str, str] | None = None,
- metadata_config: dict[str, str] | None = None,
- source_collection: str | None = "",
Review Comment:
This looks like a breaking change.
PR is still in WIP but just posting a comment to update the changelog accordingly we decided to release a breaking 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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567536996
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
@sunank200 , All the other parameters are now part of the `Spec` object. Hence, removed them.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1579126340
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
Can you change this name if its not a standard term used, p
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
Can you change this name if its not a standard term used, please
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1580869413
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
+ pinecone_host = self.conn.host
+ extras = self.conn.extra_dejson
pinecone_project_id = extras.get("project_id")
- log_level = extras.get("log_level", None)
- pinecone.init(
- api_key=api_key,
- environment=pinecone_environment,
- host=pinecone_host,
- project_name=pinecone_project_id,
- log_level=log_level,
- )
+ enable_curl_debug = extras.get("debug_curl")
+ if enable_curl_debug:
+ os.environ["PINECONE_DEBUG_CURL"] = "true"
+ return Pinecone(api_key=self.api_key, host=pinecone_host, project_id=pinecone_project_id)
+
+ def get_conn(self) -> Connection:
Review Comment:
I've changed it to a property.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1566585987
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
What about other parameters which was there previously?
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
What about other parameters which was there previously?
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
Can this method be named better?
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
Review Comment:
doc-string missing
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
What about other parameters which was there previously? Is it 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488772082
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
Regarding the pin, I did want to limit >=3.0.0 and <3.1.0. Since, <2 wouldn't work for us and 3.1 can possibly have breaking changes. Once Pinecone releases 3.1, I thought we can test them out and then change pin to `pinecone-client>3,<3.1` . Do you think its a reasonable approach?
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
Regarding the pin, I did want to limit >=3.0.0 and <3.1.0. Since, <2 wouldn't work for us and 3.1 can possibly have breaking changes. Once Pinecone releases 3.1, I thought we can test them out and then change pin to `pinecone-client>3,<3.2` . Do you think its a reasonable approach?
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567138786
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
Review Comment:
Should we do something like
```python
self.environment = environment if environment else self.conn.login
```
in `__init__` and check all these attribute in `__init__` all at once? (e.g., `all([...])`)
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "sunank200 (via GitHub)" <gi...@apache.org>.
sunank200 commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1565811272
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,80 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self,
+ conn_id: str = default_conn_name,
+ environment: str | None = None,
+ region: str | None = None,
+ api_key: str | None = None,
Review Comment:
Should this `api_key` be fetched from airflow connection for pinecone?
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1566143859
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,80 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self,
+ conn_id: str = default_conn_name,
+ environment: str | None = None,
+ region: str | None = None,
+ api_key: str | None = None,
Review Comment:
I agree, that credentials and sensitive information should be a part of the Connections, and better avoid to direct assignment it.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1580864264
##########
airflow/providers/pinecone/CHANGELOG.rst:
##########
@@ -20,6 +20,36 @@
Changelog
---------
+2.0.0
+.....
+
+Breaking changes
+~~~~~~~~~~~~~~~~
+
+.. warning::
+ This release of provider has breaking changes from previous versions. Changes are based on
+ the migration guide from pinecone - <https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b>
+
+* ``log_level`` field is removed from the Connections as it is not used by the provider anymore.
+* ``PineconeHook.get_conn`` now returns ``Connection`` object instead of ``PineconeConnection`` object. Use ``pc`` property to access the Pinecone client.
+* Following ``PineconeHook`` methods are converted from static methods to instance methods. Hence, Initialization is required to use these now:
Review Comment:
Earlier, imported pinecone module object can directly be used. Hence, staticmethod made sense. But now, we need to initialize a pinecone object. Hence, having them static methods and then again sending the connection object to all of them didn't make sense to me. hence, this change
Earlier pinecone connection creation:
```
import pinecone
pinecone.init(
api_key='XXX',
environment='us-east1-gcp'
)
```
[This](https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b) migration guide talks about this in detail
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1485570953
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -63,43 +64,40 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
Review Comment:
Thanks, I will look into this and the other comment
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488803231
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
I understand. Thanks, I'll use `pinecone-client>=3.0.0`
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488772082
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -44,7 +44,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
- - pinecone-client>=2.2.4,<3.0
+ - pinecone-client~=3.0.0
Review Comment:
Regarding the pin, I did want to limit >=3.0.0 and <3.1.0. Since, <2 wouldn't work for us and 3.1 can possibly have breaking changes. Once Pinecone releases 3.1, I thought we can test them out and then change pin to `pinecone-client~=3.0` . Do you think its a reasonable approach?
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488788371
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -63,43 +64,40 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
Review Comment:
So, pinecone environment can be something they can pass as an argument to the hook or the operator. This way, they can actually create/interact indexes in different regions without restrictions.
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1488788371
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -63,43 +64,40 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
Review Comment:
So, pinecone environment can be something they can pass as an argument to the hook or the operator. This way, they can actually create indexes in different regions without restrictions.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1565860151
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,80 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self,
+ conn_id: str = default_conn_name,
+ environment: str | None = None,
+ region: str | None = None,
+ api_key: str | None = None,
Review Comment:
So, it can be passed via the argument or the connection. I added a property to fetch it from either with order of priority being argument and then the connection
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1565860151
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,80 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self,
+ conn_id: str = default_conn_name,
+ environment: str | None = None,
+ region: str | None = None,
+ api_key: str | None = None,
Review Comment:
So, it can be passed via the argument or the connection. I added a property to fetch it from either with order of priority being argument and then the connection
https://github.com/apache/airflow/blob/8c612486ef02cfe419b0d643f53ddd5674e84556/airflow/providers/pinecone/hooks/pinecone.py#L93
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37307:
URL: https://github.com/apache/airflow/pull/37307#issuecomment-2041247952
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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
Re: [PR] [WIP] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #37307:
URL: https://github.com/apache/airflow/pull/37307#issuecomment-1937490597
> Does it backward compatible? Does it break during upgrade?
Difficult to say right now, in general bump dependency version is not a breaking changes, if all other components remain unchanged or compatible.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1567525149
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -64,43 +69,73 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
"""Return custom field behaviour."""
return {
"hidden_fields": ["port", "schema"],
- "relabeling": {"login": "Pinecone Environment", "password": "Pinecone API key"},
+ "relabeling": {
+ "login": "Pinecone Environment",
+ "host": "Pinecone Host",
+ "password": "Pinecone API key",
+ },
}
- def __init__(self, conn_id: str = default_conn_name) -> None:
+ def __init__(
+ self, conn_id: str = default_conn_name, environment: str | None = None, region: str | None = None
+ ) -> None:
self.conn_id = conn_id
- self.get_conn()
-
- def get_conn(self) -> None:
- pinecone_connection = self.get_connection(self.conn_id)
- api_key = pinecone_connection.password
- pinecone_environment = pinecone_connection.login
- pinecone_host = pinecone_connection.host
- extras = pinecone_connection.extra_dejson
+ self._environment = environment
+ self._region = region
+ self.conn = self.get_conn()
+
+ @property
+ def api_key(self) -> str:
+ key = self.conn.password
+ if not key:
+ raise LookupError("Pinecone API Key not found in connection")
+ return key
+
+ @cached_property
+ def environment(self) -> str:
+ if self._environment:
+ return self._environment
+ env = self.conn.login
+ if not env:
+ raise LookupError("Pinecone environment not found in connection")
+ return env
+
+ @cached_property
+ def region(self) -> str:
+ if self._region:
+ return self._region
+ region = self.conn.extra_dejson.get("region")
+ if not region:
+ raise LookupError("Pinecone region not found in connection")
+ return region
+
+ @cached_property
+ def pc(self) -> Pinecone:
+ """Pinecone object to interact with Pinecone."""
Review Comment:
I choose `pc` because that seems to be the variable used for pinecone connection all over their docs. It felt similar to `df` in pandas. But, I can change it if you think it should be updated.
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1578266977
##########
airflow/providers/pinecone/provider.yaml:
##########
@@ -45,7 +45,7 @@ dependencies:
# Pinecone Python SDK v3.0.0 was released at 2024-01-16 and introduce some breaking changes.
# It's crucial to adhere to the v3.0.0 Migration Guide before the upper-bound limitation can be removed.
# https://canyon-quilt-082.notion.site/Pinecone-Python-SDK-v3-0-0-Migration-Guide-056d3897d7634bf7be399676a4757c7b
Review Comment:
This note can now be removed isn't it?
--
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
Re: [PR] Pinecone provider support for `pinecone-client`>=3 [airflow]
Posted by "rawwar (via GitHub)" <gi...@apache.org>.
rawwar commented on code in PR #37307:
URL: https://github.com/apache/airflow/pull/37307#discussion_r1581639781
##########
airflow/providers/pinecone/hooks/pinecone.py:
##########
@@ -135,119 +170,106 @@ def upsert(
**kwargs,
)
- @staticmethod
+ def get_pod_spec_obj(
+ self,
+ replicas: int | None = None,
+ shards: int | None = None,
+ pods: int | None = None,
+ pod_type: str | None = "p1.x1",
+ metadata_config: dict | None = None,
+ source_collection: str | None = None,
+ environment: str | None = None,
+ ) -> PodSpec:
+ return PodSpec(
+ environment=environment or self.environment,
+ replicas=replicas,
+ shards=shards,
+ pods=pods,
+ pod_type=pod_type,
+ metadata_config=metadata_config,
+ source_collection=source_collection,
+ )
+
+ def get_serverless_spec_obj(self, cloud, region: str | None = None) -> ServerlessSpec:
+ return ServerlessSpec(cloud=cloud, region=region or self.region)
+
def create_index(
+ self,
Review Comment:
Sorry, we have separate methods for Spec objects and their docstrings talk about all the required parameters. Those two functions are right above this method. I think I'll update the docstring to mention about using the relevant methods to generate the Spec object. I guess that would clear things
--
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