Skip to content

Remove openapi_config property #369

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

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

jhamon
Copy link
Collaborator

@jhamon jhamon commented Jul 19, 2024

Problem

We want to remove the openapi_config config property because we don't want generated openapi stuff in the public interface. This is a holdover from the v2 days of pinecone.init() but has never been documented; I would be surprised if anyone is actually using it.

Solution

  • Convert deprecation warning into an exception
  • Remove unit tests relying on this deprecated option

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@jhamon jhamon requested a review from austin-denoble July 19, 2024 00:46
@jhamon jhamon marked this pull request as ready for review July 19, 2024 00:46
Comment on lines 216 to +217
if kwargs.get("openapi_config", None):
warnings.warn(
"Passing openapi_config is deprecated and will be removed in a future release. Please pass settings such as proxy_url, proxy_headers, ssl_ca_certs, and ssl_verify directly to the Pinecone constructor as keyword arguments. See the README at https://github.com/pinecone-io/pinecone-python-client for examples.",
DeprecationWarning,
raise Exception(
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so the part that checks for the kwarg of this is part of the underlying generated code implementation? That's annoying.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The whole bucket of kwargs gets passed around in various places, so to be sure nobody has formed a dependency on these internals (so we can safely refactor later) we need to explicitly check and throw right at the entry point.

@jhamon jhamon merged commit 9002584 into release-candidate/2024-07 Jul 19, 2024
83 checks passed
@jhamon jhamon deleted the jhamon/config-changes branch July 19, 2024 12:03
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