Skip to content

Python: updating pinecone client #6021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
May 8, 2024
Merged

Python: updating pinecone client #6021

merged 7 commits into from
May 8, 2024

Conversation

LazaroHurtado
Copy link
Contributor

Motivation and Context

Description

  1. Why is this change required?
    • Allow developers to use Pinecone as a memory store
  2. What problem does it solve?
    • Currently using the Pinecone memory store class throws the deprecation error message show below:
      AttributeError: init is no longer a top-level attribute of the pinecone package.
      This PR fixes this issue by updating pinecone-client to a new major version and using the new initialization.
  3. If it fixes an open issue, please link to the issue here.
    Python: PineconeMemoryStore fails to initialize with a pinecone client error #4914

Contribution Checklist

@LazaroHurtado LazaroHurtado requested a review from a team as a code owner April 28, 2024 04:24
@markwallace-microsoft markwallace-microsoft added python Pull requests for the Python Semantic Kernel memory labels Apr 28, 2024
@LazaroHurtado LazaroHurtado changed the title updating pinecone client Python: updating pinecone client Apr 28, 2024
LazaroHurtado
LazaroHurtado commented Apr 28, 2024
View reviewed changes
eavanvalkenburg
eavanvalkenburg previously requested changes Apr 29, 2024
View reviewed changes
Copy link
Member

@eavanvalkenburg eavanvalkenburg left a comment

Choose a reason for hiding this comment

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

Most important would be to start using a async client, if available, will make the whole thing more usable.

@moonbox3
Copy link
Contributor

moonbox3 commented May 7, 2024

@LazaroHurtado could you please get latest main and merge to your branch to fix the conflict?

@LazaroHurtado
Copy link
Contributor Author

LazaroHurtado commented May 8, 2024 •
edited
Loading

@moonbox3 I have merged in the latest changes from main.

@moonbox3
Copy link
Contributor

moonbox3 commented May 8, 2024

@moonbox3 I have merged in the latest changes from main.

I think something went wrong with the merge because there are now Java files in this PR. We should only be seeing your Pinecone changes. Could you check, please?

@LazaroHurtado
Copy link
Contributor Author

@moonbox3, yeah that was weird. There was a recent commit to remove that folder but I guess the merge didn't clean it up locally and it persisted. All should be good now

@moonbox3
Copy link
Contributor

moonbox3 commented May 8, 2024

In the python directory, can you please run poetry lock --no-update to fix the current errors?

The failure is:

pyproject.toml changed significantly since poetry.lock was last generated. Run poetry lock [--no-update] to fix the lock file.

@markwallace-microsoft
Copy link
Member

Py3.10 Test Coverage

Python 3.10 Test Coverage Report •
FileStmtsMissCoverMissing
TOTAL565188784% 
report-only-changed-files is enabled. No files were changed during this commit :)

Python 3.10 Unit Test Overview

Tests Skipped Failures Errors Time
1280 1 💤 0 ❌ 0 🔥 14.135s ⏱️

@LazaroHurtado
Copy link
Contributor Author

@moonbox3 Thanks for that recommendation! I ran the command and made the changes, all checks pass now

@moonbox3
Copy link
Contributor

moonbox3 commented May 8, 2024

@moonbox3 Thanks for that recommendation! I ran the command and made the changes, all checks pass now

Looking good now. Will get another pair of eyes for the other review so we can move this forward. Thanks.

@moonbox3 moonbox3 dismissed eavanvalkenburg’s stale review May 8, 2024 17:44

PR received 2 approving reviews, and previous reviewer is oof.

@moonbox3 moonbox3 added this pull request to the merge queue May 8, 2024
Merged via the queue into microsoft:main with commit 9e70e71 May 8, 2024
25 checks passed
@LazaroHurtado LazaroHurtado deleted the issue-4914 branch May 8, 2024 18:34
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@LazaroHurtado
Python: updating pinecone client (microsoft#6021)
012a898 
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  3. What problem does it solve?
  4. What scenario does it contribute to?
  5. If it fixes an open issue, please link to the issue here.
-->

### Description
1. Why is this change required?
    - Allow developers to use Pinecone as a memory store
2. What problem does it solve?
- Currently using the Pinecone memory store class throws the deprecation
error message show below:
`AttributeError: init is no longer a top-level attribute of the pinecone
package.`
This PR fixes this issue by updating `pinecone-client` to a new major
version and using the new initialization.
3. If it fixes an open issue, please link to the issue here.
    microsoft#4914 

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lazaro Hurtado <lhurtado@microsoft.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@LazaroHurtado
Python: updating pinecone client (microsoft#6021)
cd54eb7 
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  3. What problem does it solve?
  4. What scenario does it contribute to?
  5. If it fixes an open issue, please link to the issue here.
-->

### Description
1. Why is this change required?
    - Allow developers to use Pinecone as a memory store
2. What problem does it solve?
- Currently using the Pinecone memory store class throws the deprecation
error message show below:
`AttributeError: init is no longer a top-level attribute of the pinecone
package.`
This PR fixes this issue by updating `pinecone-client` to a new major
version and using the new initialization.
3. If it fixes an open issue, please link to the issue here.
    microsoft#4914 

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lazaro Hurtado <lhurtado@microsoft.com>
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
@LazaroHurtado
Python: updating pinecone client (microsoft#6021)
6ff750c 
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  3. What problem does it solve?
  4. What scenario does it contribute to?
  5. If it fixes an open issue, please link to the issue here.
-->

### Description
1. Why is this change required?
    - Allow developers to use Pinecone as a memory store
2. What problem does it solve?
- Currently using the Pinecone memory store class throws the deprecation
error message show below:
`AttributeError: init is no longer a top-level attribute of the pinecone
package.`
This PR fixes this issue by updating `pinecone-client` to a new major
version and using the new initialization.
3. If it fixes an open issue, please link to the issue here.
    microsoft#4914 

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Lazaro Hurtado <lhurtado@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliomenendez juliomenendez juliomenendez approved these changes

@moonbox3 moonbox3 moonbox3 approved these changes

@eavanvalkenburg eavanvalkenburg eavanvalkenburg left review comments

Assignees
No one assigned
Labels
memory python Pull requests for the Python Semantic Kernel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@LazaroHurtado @moonbox3 @markwallace-microsoft @juliomenendez @eavanvalkenburg