Skip to content
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.

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

Handle changes related to new API (Serverless spec) #19

Merged
merged 22 commits into from
Aug 13, 2024

Conversation

vdayanand
Copy link
Contributor

@vdayanand vdayanand commented Jun 19, 2024

ref: https://docs.pinecone.io/guides/operations/migrate-to-the-new-api

  • adds a new objects with suffix "v3" to handle the new API

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced new functions for HTTP POST and DELETE requests to enhance data manipulation capabilities.
    • Added a new file defining an interface for the Pinecone v3 API, including structures and functions for managing vector indices.
  • Improvements

    • Updated display formats for PineconeIndex and PineconeVector for clearer output.
  • Tests

    • Added a new test suite for the Pinecone v3 API to validate core functionalities, including index creation and vector operations.

Copy link

coderabbitai bot commented Jun 19, 2024

Walkthrough

The recent changes significantly enhance the Pinecone API by introducing new structures and functions, improving the display formats of existing components, and expanding HTTP interaction capabilities. The addition of the v3.jl file supports the latest API features, while updates to the test suite ensure comprehensive validation of all functionalities. These enhancements provide users with a clearer interface and an improved experience when managing vector data.

Changes

Files Change Summary
src/Pinecone.jl, src/v3.jl Enhanced display methods for existing structs, introduced new structures and functions for managing vector indices in the v3 API.
src/networking.jl Introduced new functions for HTTP POST and DELETE requests to enhance HTTP interaction capabilities.
test/runtests.jl, test/v3.jl Extended the test suite to cover new functionalities, including tests for the new v3 API features.

Poem

🐇 In the garden of code, where features bloom,
New functions leap forth, dispelling the gloom.
With v3 in hand, our tasks are a breeze,
Upserting and fetching with effortless ease.
So hop along, friends, and join in the cheer,
For Pinecone's bright future is wonderfully here! 🌼✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@vdayanand vdayanand changed the title Handle changes new API related to Serverless spec Handle changes related to new API (Serverless spec) Jun 19, 2024
@vdayanand vdayanand marked this pull request as ready for review August 7, 2024 09:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between be357a8 and b212b40.

Files selected for processing (5)
  • src/Pinecone.jl (4 hunks)
  • src/networking.jl (2 hunks)
  • src/v3.jl (1 hunks)
  • test/runtests.jl (1 hunks)
  • test/v3.jl (1 hunks)
Additional comments not posted (20)
test/runtests.jl (1)

15-15: Include new test file v3.jl.

The new test file v3.jl is included to expand the test suite. Ensure that the tests in v3.jl are comprehensive and cover all necessary scenarios for the new API.

src/networking.jl (2)

21-24: Add new function pineconeHTTPPost.

The new function pineconeHTTPPost allows sending HTTP POST requests with a specified URL, API key, and an optional body. Ensure the headers are correctly constructed and the dohttp function is properly called.


31-34: Add new function pineconeHTTPDelete.

The new function pineconeHTTPDelete allows sending HTTP DELETE requests with a specified URL and API key. Ensure the headers are correctly constructed and the dohttp function is properly called.

test/v3.jl (7)

1-3: Initialize context and create index.

The context is initialized using the API key from the environment variable, and an index is created. Ensure the API key is correctly set in the environment and the index creation parameters are appropriate.


4-17: Verify index properties.

The properties of the created index are verified. Ensure the tests accurately reflect the expected properties of the index.


19-23: Verify index statistics.

The statistics of the index are described and verified. Ensure the tests accurately reflect the expected statistics of the index.


25-33: Upsert and verify vector.

A vector is upserted into the index and its statistics are verified. Ensure the tests accurately reflect the expected behavior of the upsert operation.


35-49: Fetch and verify vector.

A vector is fetched from the index and its properties are verified. Ensure the tests accurately reflect the expected behavior of the fetch operation.


51-57: Delete and verify vector.

A vector is deleted from the index and its statistics are verified. Ensure the tests accurately reflect the expected behavior of the delete operation.


58-60: Delete and verify index.

The index is deleted and the list of indexes is verified. Ensure the tests accurately reflect the expected behavior of the delete operation.

src/v3.jl (8)

3-6: LGTM!

The PineconeContextv3 structure is correctly defined with a custom Base.show method.


7-10: LGTM!

The Serverless structure is correctly defined.


12-16: LGTM!

The PodSpec structure is correctly defined using the @kwdef macro.


18-21: LGTM!

The Status structure is correctly defined.


23-35: LGTM!

The PineconeIndexv3 structure is correctly defined with a custom Base.show method and StructTypes.StructType definitions for JSON3 compatibility.


37-47: LGTM!

The pineconeMakeURLForIndex functions are correctly defined and generate URLs based on the provided parameters.


49-66: LGTM!

The init_v3 function is correctly defined with a clear and informative docstring.


112-125: Ensure list_index_objects function is improved.

The list_indexes function relies on the list_index_objects function, which has a placeholder comment for improvement. Ensure the list_index_objects function is refined for better performance and readability.

src/Pinecone.jl (2)

20-20: LGTM!

The update to the Base.show method for PineconeIndex provides a clearer representation of the instances.


32-32: LGTM!

The

Comment on lines +153 to +168
"""
delete_index(ctx::PineconeContextv3, indexobj::PineconeIndex)

Deletes an index, returns true on successful response from Pinecone backend.

This delets a given Pinecone index, note that this is an asynch call and doesn't guarantee that on return that the index is actually deleted (yet).

# Example
```julia
Pinecone.delete_index(context, Pinecone.Index("index-to-delete"))
```
"""
function delete_index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPDelete(url, ctx.apikey)
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Implement the delete_index function.

The delete_index function is defined but not yet implemented. Consider implementing the function to delete an index.

Do you want me to implement the delete_index function or open a GitHub issue to track this task?


Handle potential errors and return an indication of success or failure.

The delete_index function does not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPDelete(url, ctx.apikey)
+    response = pineconeHTTPDelete(url, ctx.apikey)
+    if response.status == 200
+        return true
+    else
+        return false
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
delete_index(ctx::PineconeContextv3, indexobj::PineconeIndex)
Deletes an index, returns true on successful response from Pinecone backend.
This delets a given Pinecone index, note that this is an asynch call and doesn't guarantee that on return that the index is actually deleted (yet).
# Example
```julia
Pinecone.delete_index(context, Pinecone.Index("index-to-delete"))
```
"""
function delete_index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPDelete(url, ctx.apikey)
end
"""
delete_index(ctx::PineconeContextv3, indexobj::PineconeIndex)
Deletes an index, returns true on successful response from Pinecone backend.
This delets a given Pinecone index, note that this is an asynch call and doesn't guarantee that on return that the index is actually deleted (yet).
# Example
```julia
Pinecone.delete_index(context, Pinecone.Index("index-to-delete"))
```
"""
function delete_index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPDelete(url, ctx.apikey)
if response.status == 200
return true
else
return false
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, your approach of keeping the delete_index function consistent with the existing code patterns is understandable. The current implementation aligns with how other functions in the codebase handle HTTP responses. If you need further assistance, feel free to ask!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for function definitions in the codebase to understand error handling and return patterns.
ast-grep --lang julia --pattern 'function $_($$$)' --json

Length of output: 170


Script:

#!/bin/bash
# Use ripgrep to search for function definitions and their implementations in the codebase.
rg 'function ' -A 10

Length of output: 31751

Comment on lines +199 to +221
"""
delete(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, deleteall::Bool, namespace::String)

Deletes vectors indexed by ``ids``. **Warning:** The entire namespace for the vector will be deleted if deleteall=true

Returns JSON blob as ``String`` of empty object or nothing if fails.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
index = Pinecone.Index("my-index")
Pinecone.delete(context, index, ["deleteme1", "deleteme2"], false, "timnamespace")
{}
```
"""
function delete(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, deleteall::Bool=false, namespace::String="")
if(length(ids) > MAX_DELETE)
throw(ArgumentError("Max number of vectors per delete is " * string(MAX_DELETE)))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTDELETE)
postbody = Dict{String, Any}("ids"=>ids, "namespace"=>namespace, "deleteAll"=>deleteall)
response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(postbody))
return String(response.body)
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle potential errors and return an indication of success or failure.

The delete function does not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(postbody))
-    return String(response.body)
+    response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(postbody))
+    if response.status == 200
+        return String(response.body)
+    else
+        return nothing
+    end

Committable suggestion was skipped due to low confidence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, I see that the current implementation is consistent with the existing codebase, where the response body is returned as a string without explicit error handling for HTTP status codes. While maintaining consistency is important, it might be beneficial to consider enhancing error handling in future updates to improve robustness and provide clearer feedback on operation success or failure. Let me know if there's anything else you'd like to explore!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for HTTP response handling patterns in the codebase to understand consistency.
rg 'pineconeHTTP(Post|Delete|Get)' -A 5

Length of output: 6716

Comment on lines +249 to +317
"""
query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::Vector{PineconeVector}, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}())

Query an index using the given context that match ``queries`` passed in. The ``PineconeVector`` that is queries is a simple ``PineconeVector`` as described above.
Note there is an another version of ``query()`` that takes in a ``Vector{Vector{<:AbstractFloat}}`` for the ``queries`` parameter. Functionally equivalent.
Returns JSON blob as a ``String`` with the results on success, ``nothing`` on failure.

topk is an optional parameter which defaults to 10 if not specified. Do recommend using JSON3 to parse the blob.


# Example
```julia-repl
julia> testdict = Dict{String, Any}("genre"=>"documentary", "year"=>2019);
julia> pinecone_context = Pinecone.init_v3("abcdef-1234-zyxv")
abcdef-1234-zyxv / us-west1-gcp / 1c9c2f3
julia> pinecone_index = Pinecone.Index("filter-example");
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3], testdict)
PineconeVector is id: testid values: [0.3, 0.11, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.4, 0.3]meta: Dict{String, Any}("genre" => "documentary", "year" => 2019)
julia> Pinecone.query(pinecone_context, pinecone_index, [testvector], 4)
"{\"results\":[{\"matches\":[{\"id\":\"testid\",\"score\":3.98966023e-07,\"values\":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3]},{\"id\":\"C\",\"score\":0.0461001098,\"values\":[0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3]}
],\"namespace\":\"\"}]}"
```
"""
function query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::PineconeVector, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}())
#rawvectors = [queries[i].values for i in 1:length(queries)]
query(ctx, indexobj, queries.values, topk, namespace, includevalues, includemeta, filter)
end

"""
query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::Vector{Vector{T}}, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}()) where {T<:AbstractFloat}

Query an index using the given context that match ``queries`` passed in. Returns JSON blob as a ``String`` with the results on success, ``nothing`` on failure.
Note there is an alternate form for ``query()`` that takes in a ``Vector{PineconeVector}`` instead. Functionally equivalent.
topk is an optional parameter which defaults to 10 if not specified. Do recommend using JSON3 to parse the blob.

# Example
```julia-repl
julia> testdict = Dict{String, Any}("genre"=>"documentary", "year"=>2019);
julia> pinecone_context = Pinecone.init_v3("abcdef-1234-zyxv")
abcdef-1234-zyxv / us-west1-gcp / 1c9c2f3
julia> pinecone_index = Pinecone.Index("filter-example");
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3], testdict)
PineconeVector is id: testid values: [0.3, 0.11, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.4, 0.3]meta: Dict{String, Any}("genre" => "documentary", "year" => 2019)
julia> Pinecone.query(pinecone_context, pinecone_index,
[[0.2, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3], [0.2, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3]], 4)
"{\"results\":[{\"matches\":[{\"id\":\"testid\",\"score\":3.98966023e-07,\"values\":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3]},{\"id\":\"C\",\"score\":0.0461001098,\"values\":[0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3]}
],\"namespace\":\"\"}]}"
```
"""
function query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vector::Vector{T}, topk::Int64=10, namespace::String="", includevalues::Bool=false,
includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}()) where {T<:AbstractFloat}
if topk > MAX_TOPK
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK)))
end
if includevalues == true && topk > MAX_TOPK_WITH_DATA
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK_WITH_DATA) * " when including data in results"))
end
if includemeta == true && topk > MAX_TOPK_WITH_META
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK_WITH_META) * " when including meatadata in results"))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTQUERYINDEX)
body = Dict{String, Any}("topK"=>topk, "vector"=>vector, "includeValues"=>includevalues, "includeMetadata"=>includemeta, "namespace"=>namespace)
if(length(filter) > 0)
body["filter"] = filter;
end
postbody = JSON3.write(body)
response = pineconeHTTPPost(url, ctx.apikey, postbody)
return String(response.body)
end #query
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle potential errors and return an indication of success or failure.

The query functions do not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPPost(url, ctx.apikey, postbody)
-    return String(response.body)
+    response = pineconeHTTPPost(url, ctx.apikey, postbody)
+    if response.status == 200
+        return String(response.body)
+    else
+        return nothing
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::Vector{PineconeVector}, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}())
Query an index using the given context that match ``queries`` passed in. The ``PineconeVector`` that is queries is a simple ``PineconeVector`` as described above.
Note there is an another version of ``query()`` that takes in a ``Vector{Vector{<:AbstractFloat}}`` for the ``queries`` parameter. Functionally equivalent.
Returns JSON blob as a ``String`` with the results on success, ``nothing`` on failure.
topk is an optional parameter which defaults to 10 if not specified. Do recommend using JSON3 to parse the blob.
# Example
```julia-repl
julia> testdict = Dict{String, Any}("genre"=>"documentary", "year"=>2019);
julia> pinecone_context = Pinecone.init_v3("abcdef-1234-zyxv")
abcdef-1234-zyxv / us-west1-gcp / 1c9c2f3
julia> pinecone_index = Pinecone.Index("filter-example");
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3], testdict)
PineconeVector is id: testid values: [0.3, 0.11, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.4, 0.3]meta: Dict{String, Any}("genre" => "documentary", "year" => 2019)
julia> Pinecone.query(pinecone_context, pinecone_index, [testvector], 4)
"{\"results\":[{\"matches\":[{\"id\":\"testid\",\"score\":3.98966023e-07,\"values\":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3]},{\"id\":\"C\",\"score\":0.0461001098,\"values\":[0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3]}
],\"namespace\":\"\"}]}"
```
"""
function query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::PineconeVector, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}())
#rawvectors = [queries[i].values for i in 1:length(queries)]
query(ctx, indexobj, queries.values, topk, namespace, includevalues, includemeta, filter)
end
"""
query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, queries::Vector{Vector{T}}, topk::Int64=10, namespace::String="", includevalues::Bool=false, includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}()) where {T<:AbstractFloat}
Query an index using the given context that match ``queries`` passed in. Returns JSON blob as a ``String`` with the results on success, ``nothing`` on failure.
Note there is an alternate form for ``query()`` that takes in a ``Vector{PineconeVector}`` instead. Functionally equivalent.
topk is an optional parameter which defaults to 10 if not specified. Do recommend using JSON3 to parse the blob.
# Example
```julia-repl
julia> testdict = Dict{String, Any}("genre"=>"documentary", "year"=>2019);
julia> pinecone_context = Pinecone.init_v3("abcdef-1234-zyxv")
abcdef-1234-zyxv / us-west1-gcp / 1c9c2f3
julia> pinecone_index = Pinecone.Index("filter-example");
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3], testdict)
PineconeVector is id: testid values: [0.3, 0.11, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.4, 0.3]meta: Dict{String, Any}("genre" => "documentary", "year" => 2019)
julia> Pinecone.query(pinecone_context, pinecone_index,
[[0.2, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3], [0.2, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3, 0.3]], 4)
"{\"results\":[{\"matches\":[{\"id\":\"testid\",\"score\":3.98966023e-07,\"values\":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3]},{\"id\":\"C\",\"score\":0.0461001098,\"values\":[0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3,0.3]}
],\"namespace\":\"\"}]}"
```
"""
function query(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vector::Vector{T}, topk::Int64=10, namespace::String="", includevalues::Bool=false,
includemeta::Bool=false, filter::Dict{String, Any}=Dict{String,Any}()) where {T<:AbstractFloat}
if topk > MAX_TOPK
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK)))
end
if includevalues == true && topk > MAX_TOPK_WITH_DATA
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK_WITH_DATA) * " when including data in results"))
end
if includemeta == true && topk > MAX_TOPK_WITH_META
throw(ArgumentError("topk larger than largest topk available of " * string(MAX_TOPK_WITH_META) * " when including meatadata in results"))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTQUERYINDEX)
body = Dict{String, Any}("topK"=>topk, "vector"=>vector, "includeValues"=>includevalues, "includeMetadata"=>includemeta, "namespace"=>namespace)
if(length(filter) > 0)
body["filter"] = filter;
end
postbody = JSON3.write(body)
response = pineconeHTTPPost(url, ctx.apikey, postbody)
return String(response.body)
end #query
response = pineconeHTTPPost(url, ctx.apikey, postbody)
if response.status == 200
return String(response.body)
else
return nothing
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, I see that your approach aligns with the existing codebase's style, where HTTP responses are returned directly without explicit status checks. While consistency is important, you might consider adding error handling in future updates to improve robustness and provide clearer indications of success or failure. Let me know if you need any further assistance!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other functions in the codebase that use HTTP requests and check their error handling.
rg 'pineconeHTTP(Post|Get|Delete)' -A 5

Length of output: 6716

src/v3.jl Outdated
Comment on lines 104 to 110
function list_index_objects(ctx::PineconeContextv3)
response = pineconeHTTPGet("https://api.pinecone.io/indexes", ctx.apikey)
## do better here
indexes = JSON3.read(String(response.body), Dict{Symbol, Any})[:indexes]
a = JSON3.write(indexes)
JSON3.read(JSON3.write(indexes), Vector{PineconeIndexv3})
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Improve the parsing logic.

The list_index_objects function includes a placeholder comment indicating that the parsing can be improved. Consider refining the parsing logic for better performance and readability.

-    ## do better here
-    indexes = JSON3.read(String(response.body), Dict{Symbol, Any})[:indexes]
-    a = JSON3.write(indexes)
-    JSON3.read(JSON3.write(indexes), Vector{PineconeIndexv3})
+    indexes = JSON3.read(String(response.body), Vector{PineconeIndexv3})
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function list_index_objects(ctx::PineconeContextv3)
response = pineconeHTTPGet("https://api.pinecone.io/indexes", ctx.apikey)
## do better here
indexes = JSON3.read(String(response.body), Dict{Symbol, Any})[:indexes]
a = JSON3.write(indexes)
JSON3.read(JSON3.write(indexes), Vector{PineconeIndexv3})
end
function list_index_objects(ctx::PineconeContextv3)
response = pineconeHTTPGet("https://api.pinecone.io/indexes", ctx.apikey)
indexes = JSON3.read(String(response.body), Vector{PineconeIndexv3})
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This suggestion doesn't work

Copy link

Choose a reason for hiding this comment

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

@vdayanand, the issue arises because the JSON response is nested, with the "indexes" key containing the array of index objects. Here's a corrected approach:

function list_index_objects(ctx::PineconeContextv3)
    response = pineconeHTTPGet("https://api.pinecone.io/indexes", ctx.apikey)
    indexes_dict = JSON3.read(String(response.body), Dict{Symbol, Any})
    indexes = JSON3.read(JSON3.write(indexes_dict[:indexes]), Vector{PineconeIndexv3})
end

This approach first extracts the "indexes" key from the response before converting it into a Vector{PineconeIndexv3}. Let me know if this resolves the issue!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the structure of the JSON response from the Pinecone API to understand the issue.
echo '{
  "indexes": [
    {
      "dimension": 128,
      "host": "example.pinecone.io",
      "metric": "cosine",
      "name": "example-index",
      "spec": {
        "serverless": {
          "cloud": "aws",
          "region": "us-east-1"
        }
      },
      "status": {
        "ready": true,
        "state": "active"
      }
    }
  ]
}' | jq '.indexes | map({dimension, host, metric, name, spec, status})'

Length of output: 716

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok applied the change

Copy link

Choose a reason for hiding this comment

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

@vdayanand, glad to hear that the change worked for you! If you need any more assistance, feel free to ask.

  (\(\ 
  ( -.-)
o_(")(")

Comment on lines +128 to +145
"""
Index(ctx::PineconeContextv3, indexname::String)

Returns a PineconeIndex type which is used for query() and upsert() of data against a specific index.

# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
julia> Pinecone.Index("my-index")
PineconeIndex connected to my-index
```
"""

function Index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPGet(url, ctx.apikey)
return JSON3.read(String(response.body), PineconeIndexv3)
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle the response and return an indication of success or failure.

The Index function does not handle the response or return any indication of success or failure. Consider adding response handling and returning a boolean or status message.

-    response = pineconeHTTPGet(url, ctx.apikey)
-    return JSON3.read(String(response.body), PineconeIndexv3)
+    response = pineconeHTTPGet(url, ctx.apikey)
+    if response.status == 200
+        return JSON3.read(String(response.body), PineconeIndexv3)
+    else
+        return nothing
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
Index(ctx::PineconeContextv3, indexname::String)
Returns a PineconeIndex type which is used for query() and upsert() of data against a specific index.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
julia> Pinecone.Index("my-index")
PineconeIndex connected to my-index
```
"""
function Index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPGet(url, ctx.apikey)
return JSON3.read(String(response.body), PineconeIndexv3)
end
"""
Index(ctx::PineconeContextv3, indexname::String)
Returns a PineconeIndex type which is used for query() and upsert() of data against a specific index.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
julia> Pinecone.Index("my-index")
PineconeIndex connected to my-index
```
"""
function Index(ctx::PineconeContextv3, indexname::String)
url = pineconeMakeURLForIndex(indexname)
response = pineconeHTTPGet(url, ctx.apikey)
if response.status == 200
return JSON3.read(String(response.body), PineconeIndexv3)
else
return nothing
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, I see that your approach aligns with the existing codebase, where response status handling is not explicitly done in many functions. Keeping it consistent makes sense in this context. If you have any other questions or need further assistance, feel free to ask!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for functions that interact with the Pinecone API to check response handling.
ast-grep --lang julia --pattern 'function $_($$$) $$_ = pineconeHTTPGet($$_, $$_)' -A 10
ast-grep --lang julia --pattern 'function $_($$$) $$_ = pineconeHTTPPost($$_, $$_, $$_)' -A 10
ast-grep --lang julia --pattern 'function $_($$$) $$_ = pineconeHTTPDelete($$_, $$_)' -A 10

Length of output: 614


Script:

#!/bin/bash
# Search for functions that interact with the Pinecone API using ripgrep.
rg 'pineconeHTTPGet' -A 10
rg 'pineconeHTTPPost' -A 10
rg 'pineconeHTTPDelete' -A 10

Length of output: 13303


Learnings added
Learnt from: vdayanand
PR: tullytim/Pinecone.jl#19
File: src/v3.jl:126-143
Timestamp: 2024-08-08T06:00:11.353Z
Learning: In the Pinecone.jl codebase, many functions interacting with the Pinecone API do not handle HTTP response statuses explicitly. They often return or parse the response body directly.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +147 to +151
function describe_index_stats(ctx::PineconeContextv3, indexobj::PineconeIndexv3)
url = pineconeMakeURLForIndex(indexobj, ENDPOINTDESCRIBEINDEXSTATS)
response = pineconeHTTPGet(url, ctx.apikey)
return String(response.body)
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle potential errors and return an indication of success or failure.

The describe_index_stats function does not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPGet(url, ctx.apikey)
-    return String(response.body)
+    response = pineconeHTTPGet(url, ctx.apikey)
+    if response.status == 200
+        return String(response.body)
+    else
+        return nothing
+    end

Committable suggestion was skipped due to low confidence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, I see that the current implementation is consistent with how similar functions handle responses in the codebase. It might be worth considering enhanced error handling in the future to improve robustness, but maintaining consistency is understandable for now. Let me know if there's anything else you'd like to explore!

🐇

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for functions in the codebase that make HTTP requests and check their error handling practices.
ast-grep --lang julia --pattern 'function $_($$$) {
  $$$
  response = pineconeHTTPGet($_, $_)
  $$$
}'

Length of output: 216


Script:

#!/bin/bash
# Search for functions using `pineconeHTTPGet` and include context lines to understand error handling.
rg 'pineconeHTTPGet' -A 5

Length of output: 3348

Comment on lines +170 to +197
"""
upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")

upserts a Julia Vector of type PineconeVector using the given PineconeContextv3 and PineconeIndexv3 with an optional namespace (Defaults to not being applied to query if not passed.)
On success returns a JSON blob as a String type, and nothing if it fails.
This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.

# Example
```julia-repl
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3])
context = Pinecone.init_v3("abcd-123456-zyx")
index = Pinecone.Index("myindex")
result = Pinecone.upsert(context, index, [testvector], "testnamespace")
```
"""
function upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")
if(length(vectors) > MAX_UPSERT_VECTORS)
throw(ArgumentError("Max number of vectors per upsert is " * string(MAX_UPSERT_VECTORS)))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTUPSERT)
body = Dict{String, Any}("vectors" => vectors)
if namespace !== nothing && namespace != ""
body["namespace"] = namespace
end
postbody = JSON3.write(body)
response = pineconeHTTPPost(url, ctx.apikey, postbody)
return String(response.body)
end #upsert
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle potential errors and return an indication of success or failure.

The upsert function does not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPPost(url, ctx.apikey, postbody)
-    return String(response.body)
+    response = pineconeHTTPPost(url, ctx.apikey, postbody)
+    if response.status == 200
+        return String(response.body)
+    else
+        return nothing
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")
upserts a Julia Vector of type PineconeVector using the given PineconeContextv3 and PineconeIndexv3 with an optional namespace (Defaults to not being applied to query if not passed.)
On success returns a JSON blob as a String type, and nothing if it fails.
This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.
# Example
```julia-repl
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3])
context = Pinecone.init_v3("abcd-123456-zyx")
index = Pinecone.Index("myindex")
result = Pinecone.upsert(context, index, [testvector], "testnamespace")
```
"""
function upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")
if(length(vectors) > MAX_UPSERT_VECTORS)
throw(ArgumentError("Max number of vectors per upsert is " * string(MAX_UPSERT_VECTORS)))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTUPSERT)
body = Dict{String, Any}("vectors" => vectors)
if namespace !== nothing && namespace != ""
body["namespace"] = namespace
end
postbody = JSON3.write(body)
response = pineconeHTTPPost(url, ctx.apikey, postbody)
return String(response.body)
end #upsert
"""
upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")
upserts a Julia Vector of type PineconeVector using the given PineconeContextv3 and PineconeIndexv3 with an optional namespace (Defaults to not being applied to query if not passed.)
On success returns a JSON blob as a String type, and nothing if it fails.
This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.
# Example
```julia-repl
julia> testvector = Pinecone.PineconeVector("testid", [0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3])
context = Pinecone.init_v3("abcd-123456-zyx")
index = Pinecone.Index("myindex")
result = Pinecone.upsert(context, index, [testvector], "testnamespace")
```
"""
function upsert(ctx::PineconeContextv3, indexobj::PineconeIndexv3, vectors::Vector{PineconeVector}, namespace::String="")
if(length(vectors) > MAX_UPSERT_VECTORS)
throw(ArgumentError("Max number of vectors per upsert is " * string(MAX_UPSERT_VECTORS)))
end
url = pineconeMakeURLForIndex(indexobj, ENDPOINTUPSERT)
body = Dict{String, Any}("vectors" => vectors)
if namespace !== nothing && namespace != ""
body["namespace"] = namespace
end
postbody = JSON3.write(body)
response = pineconeHTTPPost(url, ctx.apikey, postbody)
if response.status == 200
return String(response.body)
else
return nothing
end
end #upsert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, your approach to handling the upsert function is consistent with the existing codebase, where functions typically return the response body directly without explicit error handling. If there's a future decision to standardize error handling across the codebase, it might be worth revisiting this approach. For now, your implementation aligns well with the current style.

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for functions that handle HTTP responses in the codebase to check for consistency in error handling.
ast-grep --lang julia --pattern $'function $_($$$) {
  $$$
  response = $_
  $$$
}'

Length of output: 195


Script:

#!/bin/bash
# Use ripgrep to search for HTTP response handling in Julia files to check for consistency in error handling.
rg 'response = pineconeHTTP' --glob '*.jl' -A 5

Length of output: 5339

Comment on lines +68 to +102
"""
create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")

Creates an index with a given PineconeContextv3, which can be accessed by a call to init(), the name of the index, and the number of dimensions.

This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.

# Example
```julia
context = Pinecone.init_v3("abcd-123456-zyx")
result = Pinecone.create_index(ctx, "my-index", 2, "euclidean")
```
"""
function create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")
dimension = dimension
if(dimension > MAX_DIMS)
throw(ArgumentError("Creating index larger than max dimension size of " * string(MAX_DIMS)))
end
metric = metric
if(metric != "cosine" && metric != "dotproduct" && metric != "euclidean")
throw(ArgumentError("Invalid index type. Type must be one of 'euclidean', 'cosine', or 'dotproduct'."))
end
body = Dict("dimension"=>dimension,
"metric"=>metric,
"name"=>indexname,
"spec"=>Dict(
"serverless"=>Dict(
"cloud"=>cloud,
"region"=>region
)
)
)
url = pineconeMakeURLForIndex()
response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(body))
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle the response and return an indication of success or failure.

The create_index function does not handle the response or return any indication of success or failure. Consider adding response handling and returning a boolean or status message.

-    response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(body))
+    response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(body))
+    if response.status == 200
+        return true
+    else
+        return false
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")
Creates an index with a given PineconeContextv3, which can be accessed by a call to init(), the name of the index, and the number of dimensions.
This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.
# Example
```julia
context = Pinecone.init_v3("abcd-123456-zyx")
result = Pinecone.create_index(ctx, "my-index", 2, "euclidean")
```
"""
function create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")
dimension = dimension
if(dimension > MAX_DIMS)
throw(ArgumentError("Creating index larger than max dimension size of " * string(MAX_DIMS)))
end
metric = metric
if(metric != "cosine" && metric != "dotproduct" && metric != "euclidean")
throw(ArgumentError("Invalid index type. Type must be one of 'euclidean', 'cosine', or 'dotproduct'."))
end
body = Dict("dimension"=>dimension,
"metric"=>metric,
"name"=>indexname,
"spec"=>Dict(
"serverless"=>Dict(
"cloud"=>cloud,
"region"=>region
)
)
)
url = pineconeMakeURLForIndex()
response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(body))
end
"""
create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")
Creates an index with a given PineconeContextv3, which can be accessed by a call to init(), the name of the index, and the number of dimensions.
This function returns a JSON blob as a string, or nothing if it failed. Do recommend using JSON3 to parse the blob.
# Example
```julia
context = Pinecone.init_v3("abcd-123456-zyx")
result = Pinecone.create_index(ctx, "my-index", 2, "euclidean")
```
"""
function create_index(ctx::PineconeContextv3, indexname::String, dimension::Int, metric::String="cosine", cloud::String="aws", region::String="us-east-1")
dimension = dimension
if(dimension > MAX_DIMS)
throw(ArgumentError("Creating index larger than max dimension size of " * string(MAX_DIMS)))
end
metric = metric
if(metric != "cosine" && metric != "dotproduct" && metric != "euclidean")
throw(ArgumentError("Invalid index type. Type must be one of 'euclidean', 'cosine', or 'dotproduct'."))
end
body = Dict("dimension"=>dimension,
"metric"=>metric,
"name"=>indexname,
"spec"=>Dict(
"serverless"=>Dict(
"cloud"=>cloud,
"region"=>region
)
)
)
url = pineconeMakeURLForIndex()
response = pineconeHTTPPost(url, ctx.apikey, JSON3.write(body))
if response.status == 200
return true
else
return false
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, your approach of handling the response in the create_index function is indeed consistent with the existing codebase. Many similar functions also return the response body as a string without explicitly checking the status code. If you plan to maintain consistency, no changes are needed. However, if you decide to improve response handling, it might be worth considering a broader refactor across all such functions. Let me know if you need any further assistance!

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for functions in the codebase that interact with the Pinecone API and check their response handling.
ast-grep --lang julia --pattern 'function $_($_, $_, $$$)' --json | jq -r '.matches[].text' | grep -E 'pineconeHTTP(Post|Delete|Get)'

Length of output: 246


Script:

#!/bin/bash
# Use rg to search for function definitions that call pineconeHTTPPost, pineconeHTTPDelete, or similar functions.
rg -A 5 'pineconeHTTP(Post|Delete|Get)' --type julia

Length of output: 6693

Comment on lines +223 to +247
"""
fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String)

Fetches vectors based on the vector ids for each vector, provided as the ``ids`` array for a given namespace. .
Note that namespace is optional, and if not provided defaults to not using it to filter query results by namespace.
Returns JSON blob as ``String`` show below, or ``nothing`` on failure.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
index = Pinecone.Index("my-index")
Pinecone.fetch(context, index, ["testid", "testid2"], "testnamespace")
{"vectors":{"testid":{"id":"testid","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"year":2019,"genre":"documentary"}},
"testid2":{"id":"testid2","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"genre":"documentary","year":2019}}},"namespace":"testnamespace"}
```
"""
function fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String="")
if(length(ids) > MAX_FETCH)
throw(ArgumentError("Max number of vectors per fetch is " * string(MAX_FETCH)))
end
renamedids = ["ids=$row" for row in ids]
urlargs = "?" * join(renamedids, "&") * "&namespace=$namespace"
url = pineconeMakeURLForIndex(indexobj, ENDPOINTFETCH) * urlargs
response = pineconeHTTPGet(url, ctx.apikey)
return String(response.body)
end
Copy link

@coderabbitai coderabbitai bot Aug 7, 2024

Choose a reason for hiding this comment

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

Handle potential errors and return an indication of success or failure.

The fetch function does not handle potential errors or return any indication of success or failure. Consider adding error handling and returning a boolean or status message.

-    response = pineconeHTTPGet(url, ctx.apikey)
-    return String(response.body)
+    response = pineconeHTTPGet(url, ctx.apikey)
+    if response.status == 200
+        return String(response.body)
+    else
+        return nothing
+    end
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""
fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String)
Fetches vectors based on the vector ids for each vector, provided as the ``ids`` array for a given namespace. .
Note that namespace is optional, and if not provided defaults to not using it to filter query results by namespace.
Returns JSON blob as ``String`` show below, or ``nothing`` on failure.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
index = Pinecone.Index("my-index")
Pinecone.fetch(context, index, ["testid", "testid2"], "testnamespace")
{"vectors":{"testid":{"id":"testid","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"year":2019,"genre":"documentary"}},
"testid2":{"id":"testid2","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"genre":"documentary","year":2019}}},"namespace":"testnamespace"}
```
"""
function fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String="")
if(length(ids) > MAX_FETCH)
throw(ArgumentError("Max number of vectors per fetch is " * string(MAX_FETCH)))
end
renamedids = ["ids=$row" for row in ids]
urlargs = "?" * join(renamedids, "&") * "&namespace=$namespace"
url = pineconeMakeURLForIndex(indexobj, ENDPOINTFETCH) * urlargs
response = pineconeHTTPGet(url, ctx.apikey)
return String(response.body)
end
"""
fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String)
Fetches vectors based on the vector ids for each vector, provided as the ``ids`` array for a given namespace. .
Note that namespace is optional, and if not provided defaults to not using it to filter query results by namespace.
Returns JSON blob as ``String`` show below, or ``nothing`` on failure.
# Example
```julia-repl
julia> context = Pinecone.init_v3("asdf-1234-zyxv")
index = Pinecone.Index("my-index")
Pinecone.fetch(context, index, ["testid", "testid2"], "testnamespace")
{"vectors":{"testid":{"id":"testid","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"year":2019,"genre":"documentary"}},
"testid2":{"id":"testid2","values":[0.3,0.11,0.3,0.3,0.3,0.3,0.3,0.3,0.4,0.3],"metadata":{"genre":"documentary","year":2019}}},"namespace":"testnamespace"}
```
"""
function fetch(ctx::PineconeContextv3, indexobj::PineconeIndexv3, ids::Array{String}, namespace::String="")
if(length(ids) > MAX_FETCH)
throw(ArgumentError("Max number of vectors per fetch is " * string(MAX_FETCH)))
end
renamedids = ["ids=$row" for row in ids]
urlargs = "?" * join(renamedids, "&") * "&namespace=$namespace"
url = pineconeMakeURLForIndex(indexobj, ENDPOINTFETCH) * urlargs
response = pineconeHTTPGet(url, ctx.apikey)
if response.status == 200
return String(response.body)
else
return nothing
end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have kept it consistent with how its handled in the existing code

Copy link

Choose a reason for hiding this comment

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

@vdayanand, I see that the approach in the fetch function is consistent with other functions in the codebase, which also don't explicitly handle errors or return success/failure indications. Maintaining consistency is important. However, consider enhancing error handling in future updates to improve robustness. If you need any assistance with this, feel free to reach out!

(。•̀ᴗ-)✧

Analysis chain
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for function definitions in src/v3.jl that handle HTTP responses.
ast-grep --lang julia --pattern $'function $_($$$)\n    $$$\n    response = $_\n    $$$\nend' src/v3.jl

Length of output: 211


Script:

#!/bin/bash
# Search for function definitions in src/v3.jl that handle HTTP responses.
rg 'function \w+\(.*\)' -A 10 src/v3.jl | grep -B 10 'response ='

Length of output: 3065

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b212b40 and 6a7ec49.

Files selected for processing (1)
  • src/v3.jl (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/v3.jl

@vdayanand
Copy link
Contributor Author

@tullytim could you please review the changes ?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6a7ec49 and 50ce2be.

Files selected for processing (1)
  • src/v3.jl (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • src/v3.jl

@vdayanand
Copy link
Contributor Author

@tullytim I have fixed the CI failure. Could you retrigger it again?

@vdayanand
Copy link
Contributor Author

The current failure seems to related to invalid API key in CI as it works fine locally?

 """
HTTP/1.1 401 Unauthorized
x-pinecone-api-version: 2024-04
X-Cloud-Trace-Context: 6168af481f6840608399c5de39fa395a
Date: Sat, 10 Aug 2024 22:34:23 GMT
Content-Type: text/html
Server: Google Frontend
Content-Length: 15
Via: 1.1 google
Alt-Svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000

Invalid API Key""")```

@tullytim
Copy link
Owner

I'll rerun it with a new api key

@tullytim tullytim merged commit aa613c5 into tullytim:main Aug 13, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants