Skip to content

Bumping pinecone sdk to 7.0.0 to improve podspec handling#50868

Merged
amoghrajesh merged 5 commits intoapache:mainfrom
amoghrajesh:fix-pinecone-ci
May 21, 2025
Merged

Bumping pinecone sdk to 7.0.0 to improve podspec handling#50868
amoghrajesh merged 5 commits intoapache:mainfrom
amoghrajesh:fix-pinecone-ci

Conversation

@amoghrajesh
Copy link
Contributor

@amoghrajesh amoghrajesh commented May 21, 2025

Due to pinecone bump to 7.0.0 as in https://github.com/apache/airflow/actions/runs/15151908599/job/42599727921, the argument type has changed leading to failures like:

providers/pinecone/src/airflow/providers/pinecone/hooks/pinecone.py:206: error:
Argument "pod_type" to "PodSpec" has incompatible type "Optional[str]"; expected
"Union[PodType, str]"  [arg-type]
                pod_type=pod_type,
                         ^~~~~~~~
Found 1 error in 1 file (checked 3787 source files)

Removing Optional[str] and making it str with a default. Also adding type[ignore]

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@amoghrajesh amoghrajesh added the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label May 21, 2025
@amoghrajesh amoghrajesh self-assigned this May 21, 2025
eladkal
eladkal reviewed May 21, 2025
View reviewed changes
@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 21, 2025

@uranusjr @gopidesupavan @eladkal tried handling it better. LMK what you think

eladkal
eladkal approved these changes May 21, 2025
View reviewed changes
Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 21, 2025

@eladkal should i bump the pinecone provider to 7.0.0 and up?

@eladkal
Copy link
Contributor

eladkal commented May 21, 2025

@eladkal should i bump the pinecone provider to 7.0.0 and up?

This is a bug fix i don't think anything needs to be bumped.
pinecone provider latest release is 2.3 why 7?

@eladkal
Copy link
Contributor

eladkal commented May 21, 2025

Ah you are referring to the pinecone sdk.
If the fix isn't backward compatible then we should bump the min version of the sdk

@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 21, 2025

Yeah doesn't seem that it is. They do the same in their code:

        object.__setattr__(
            self, "pod_type", pod_type.value if isinstance(pod_type, PodType) else str(pod_type)
        )

Let me bump it

@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 21, 2025

@eladkal bumped it. Looks ok?

@eladkal
Copy link
Contributor

eladkal commented May 21, 2025

Yes, lets please update the commit message to notify about the version bump. The mypy issue is secondary.

@amoghrajesh amoghrajesh changed the title Fixing mypy checks on pinecone provider Bumping pinecone sdk to 7.0.0 to improve podspec handling May 21, 2025
@amoghrajesh amoghrajesh merged commit 4af10c9 into apache:main May 21, 2025
64 checks passed
@github-actions
Copy link

github-actions bot commented May 21, 2025

Backport failed to create: v3-0-test. View the failure log Run details

Status Branch Result
❌ v3-0-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 4af10c9 v3-0-test

This should apply the commit to the v3-0-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

@potiuk
Copy link
Member

potiuk commented May 21, 2025

Ideally we should not need to cherry-pick that one if it's just a mypy check.

@potiuk potiuk removed the backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch label May 21, 2025
@amoghrajesh
Copy link
Contributor Author

amoghrajesh commented May 21, 2025

@potiuk its a bump to the pinecone version used. But shouldn't be cherry picked imo

@eladkal
Copy link
Contributor

eladkal commented May 21, 2025

We do need it in v3 branch for constraints generation

@potiuk
Copy link
Member

potiuk commented May 21, 2025

We do need it in v3 branch for constraints generation

Not any more after we split the providers to separate pyproject.toml files - it should only be (rarely) needed if the changes in dependencies are conflicting and in "preinstalled" dependencies like FAB. Generally we should only cherry-pick provider dependency changes if v3-0-test is broken without it.

@potiuk
Copy link
Member

potiuk commented May 21, 2025

Why? The pypi constraints are generally using released providers from PyPI -> if thos providers can be installed with 3.0.* airflow, all is fine.

@eladkal
Copy link
Contributor

eladkal commented May 21, 2025

Why? The pypi constraints are generally using released providers from PyPI -> if thos providers can be installed with 3.0.* airflow, all is fine.

But we could have a case that pypi backtrack and uses older versions which are under the lower limit. So there is a possibility that generating the constraints will work but it would produce unsupported versions.

@potiuk
Copy link
Member

potiuk commented May 21, 2025 •
edited
Loading

But we could have a case that pypi backtrack and uses older versions which are under the lower limit. So there is a possibility that generating the constraints will work but it would produce unsupported versions.

Unlikely - but even if it does, it does not matter as we are not running provider tests in release branches. It's only a problem if the image cannot be built in 3.0-test due to conflicts

nailo2c pushed a commit to nailo2c/airflow that referenced this pull request May 21, 2025
@amoghrajesh @nailo2c
Bumping pinecone sdk to 7.0.0 to improve podspec handling (apache#50868)
345c5d4 
* Fixing mypy checks on pinecone provider

* bump pinecone to 7.0.0
dadonnelly316 pushed a commit to dadonnelly316/airflow that referenced this pull request May 26, 2025
@amoghrajesh @dadonnelly316
Bumping pinecone sdk to 7.0.0 to improve podspec handling (apache#50868)
11e42c0 
* Fixing mypy checks on pinecone provider

* bump pinecone to 7.0.0
sanederchik pushed a commit to sanederchik/airflow that referenced this pull request Jun 7, 2025
@amoghrajesh @sanederchik
Bumping pinecone sdk to 7.0.0 to improve podspec handling (apache#50868)
00b5ef9 
* Fixing mypy checks on pinecone provider

* bump pinecone to 7.0.0
@eladkal eladkal mentioned this pull request Jun 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@uranusjr uranusjr uranusjr left review comments

@gopidesupavan gopidesupavan gopidesupavan approved these changes

@vatsrahul1001 vatsrahul1001 vatsrahul1001 approved these changes

@eladkal eladkal eladkal approved these changes

@potiuk potiuk Awaiting requested review from potiuk

Assignees

@amoghrajesh amoghrajesh

Labels

area:providers provider:pinecone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@amoghrajesh @eladkal @potiuk @uranusjr @gopidesupavan @vatsrahul1001