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

Add control plane to SDK #14

Merged
merged 8 commits into from
Mar 15, 2024
Merged

Add control plane to SDK #14

merged 8 commits into from
Mar 15, 2024

Conversation

haruska
Copy link
Contributor

@haruska haruska commented Mar 12, 2024

This PR adds the Control Plane REST API calls for managing Indexes and Collections.

Similar to the codegen for the Data Plane gRPC calls, this generates code from the OpenAPI Spec for the Control Plane REST calls. These calls are then wrapped in the SDK to reduce the surface area of the SDK to support.

haruska added 2 commits March 12, 2024 16:12
Wrap the generated code for CRUDL operations on Indexes.
Adds CRUDL operations available on Collections in the SDK. Provides
a thin wrapper around the generated code.
@haruska haruska force-pushed the add_control_plane branch 2 times, most recently from 348ff5d to c65c140 Compare March 12, 2024 21:23
Testing still requires hitting Pinecone with known indexes created.
However, this is simplified to just need the API Key and names of
the indexes. The rest is looked up by the control plane calls.
@haruska haruska force-pushed the add_control_plane branch from c65c140 to cb2e9ee Compare March 12, 2024 21:28
Copy link
Contributor

@austin-denoble austin-denoble left a comment

Choose a reason for hiding this comment

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

Nice work, I think overall this looks good to me. I've left some comments and questions but I don't think there's necessarily anything blocking here. Thanks a lot for getting the testing suite off the ground as part of this work.

I still need to find some time to pull the repo down and get set up to play around locally, but I feel like everything here makes sense.

README.md Show resolved Hide resolved
apis/openapi/control/v1/control_v1.yaml Outdated Show resolved Hide resolved
pinecone/client.go Show resolved Hide resolved
pinecone/client.go Show resolved Hide resolved
pinecone/client.go Show resolved Hide resolved
apis/openapi/control/v1/control_v1.yaml Outdated Show resolved Hide resolved
pinecone/client.go Show resolved Hide resolved
pinecone/client.go Show resolved Hide resolved
"io"
"net/http"
)

type Client struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think probably the Client struct should be lowercase/private client to force people to go through the NewClient constructor method.

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 believe Client must be exported. The fields apiKey and restClient however are not exported. The only real way to get a valid client is through NewClient

pinecone/client.go Outdated Show resolved Hide resolved
@haruska haruska merged commit f391ac7 into main Mar 15, 2024
3 checks passed
@haruska haruska deleted the add_control_plane branch March 15, 2024 13:49
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.

4 participants