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

Generate code based on 2024-10 api spec #158

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

rohanshah18
Copy link
Contributor

@rohanshah18 rohanshah18 commented Oct 18, 2024

Problem

Generate code based on 2024-10 api spec.

Solution

OpenAPI based code was regenerated using the 2024-10 api sec which resulted in the following changes:

  1. Updated build-oas.sh to include all control plane, data plane, and inference modules.
  2. Inference is now broken out of control plane so the underlying inference code generated by openAPI spec is now updated. This also resulted in instantiating a separate ApiClient which is a part of inference module now instead of the shared ApiClient class of control plane. Also as a part of this refactoring, the customOkHttpClient is now a part of the PineconeConfig class, so once the customOkHttpClient is set by the user at the time of instantiating Pinecone using its builder class, it'll be stored in the PineconeConfig instance which can further be leveraged by the Inference api calls as well.
  3. Control and data plane modules are now prepended with db_ so this resulted in updating import statements for a lot of existing classes.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

Since no new functionality is added, the existing integration tests should run fine.

@@ -3,7 +3,7 @@
set -eux -o pipefail

version=$1 # e.g. 2024-07
modules=("control")
modules=("db_control" "db_data" "inference")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. replaced control with db_control
  2. added db_data for bulk import (actual feature is going to be a part of the next PR).
  3. added inference which is now broken out of control plane

@@ -5,7 +5,7 @@
import io.pinecone.clients.Pinecone;
import io.pinecone.exceptions.PineconeException;
import io.pinecone.proto.DescribeIndexStatsResponse;
import org.openapitools.control.client.model.*;
import org.openapitools.db_control.client.model.*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

example of updating import statement since db_ is now appended to control and data modules.

import org.openapitools.control.client.model.EmbeddingsList;
import io.pinecone.configs.PineconeConfig;
import okhttp3.OkHttpClient;
import org.openapitools.inference.client.ApiClient;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

example of updating import statement since inference is broken out of control module

@@ -1033,14 +1033,14 @@ public Builder withProxy(String proxyHost, int proxyPort) {
* @return A new {@link Pinecone} instance configured based on the builder parameters.
*/
public Pinecone build() {
PineconeConfig config = new PineconeConfig(apiKey, sourceTag, proxyConfig);
PineconeConfig config = new PineconeConfig(apiKey, sourceTag, proxyConfig, customOkHttpClient);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

customOkHttpClient being passed to the PineconeConfig constructor at the time of initializing PineconeConfig in Pinecone class

}

/**
* Constructs a {@link PineconeConfig} instance with the specified API key, source tag, control plane proxy
* configuration, and data plane proxy configuration.
* Constructs a {@link PineconeConfig} instance with the specified API key, source tag, a HTTP proxy configuration,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

customOkHttpClient is now a member of PineconeConfig. It makes a lot of sense since gRPC custom channel is already a part of this config class.

@rohanshah18 rohanshah18 marked this pull request as ready for review October 18, 2024 18:22
Copy link
Contributor

@ssmith-pc ssmith-pc left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for separating the API update from the rerank feature impl

@rohanshah18 rohanshah18 changed the base branch from main to rshah/release-candidate/2024-10 October 18, 2024 18:40
@rohanshah18 rohanshah18 merged commit 0b3defd into rshah/release-candidate/2024-10 Oct 18, 2024
7 checks passed
@rohanshah18 rohanshah18 deleted the rshah/generateCode branch October 18, 2024 18:40
rohanshah18 added a commit that referenced this pull request Oct 24, 2024
## Problem

Generate code based on 2024-10 api spec.

## Solution
OpenAPI based code was regenerated using the 2024-10 api sec which
resulted in the following changes:
1. Updated `build-oas.sh` to include all control plane, data plane, and
inference modules.
2. Inference is now broken out of control plane so the underlying
`inference` code generated by openAPI spec is now updated. This also
resulted in instantiating a separate `ApiClient` which is a part of
inference module now instead of the shared `ApiClient` class of control
plane. Also as a part of this refactoring, the `customOkHttpClient` is
now a part of the `PineconeConfig` class, so once the
`customOkHttpClient` is set by the user at the time of instantiating
`Pinecone` using its builder class, it'll be stored in the
`PineconeConfig` instance which can further be leveraged by the
`Inference` api calls as well.
3. Control and data plane modules are now prepended with `db_` so this
resulted in updating import statements for a lot of existing classes.

## Type of Change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Infrastructure change (CI configs, etc)
- [ ] Non-code change (docs, etc)
- [ ] None of the above: (explain here)

## Test Plan

Since no new functionality is added, the existing integration tests
should run fine.
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