Commit 6fa0447
authored
Revise proxy configuration, add integration testing (#325)
## Problem
We want to expose proxy configuration fields as top-level config params
and move away from users passing an `OpenApiConfiguration` instance with
`openapi_config` (as was done in 2.x versions of the sdk). Passing these
objects is not something we documented or had shown as a named
configuration param, but is something people assumed would work as a
hangover from the way things were done in the old client.
Currently OpenApi generated code is part of our implementation, but we
may want to swap out something else in the future so it doesn't make
sense to use this `OpenApiConfiguration` object as part of our public
interface. And since the openapi config object also needs to be
configured with api key and host, it competes with these other config
params in a way that is pretty confusing.
## Solution
- Add new configuration parameters for `proxy_url`, `proxy_headers`,
`ssl_ca_certs`, and `ssl_verify`.
- Document the use of these properties in the README and doc strings
- Emit a warning message when users are passing `openapi_config`
encouraging them to use the new properties.
- Implement integration tests that run with
[mitmproxy](https://mitmproxy.org/) docker image. For now these are only
run in local development.
## Usage
```python
from pinecone import Pinecone
import urllib3 import make_headers
pc = Pinecone(
api_key="YOUR_API_KEY",
proxy_url='https://your-proxy.com',
proxy_headers=make_headers(proxy_basic_auth='username:password'),
ssl_ca_certs='path/to/cert-bundle.pem'
)
```
This will work, but should emit a deprecation warning:
```python
from pinecone import Pinecone
from pinecone.core.client.configuration import OpenApiConfiguration
import urllib3 import make_headers
config = OpenApiConfiguration()
config.ssl_ca_certs = 'path/to/cert-bundle.pem'
config.proxy_headers = make_headers(proxy_basic_auth='username:password')
config.proxy_url = 'https://your-proxy.com'
pc = Pinecone(
api_key="YOUR_API_KEY",
openapi_config=config
) # emits deprecation notice
```
## Future work
We will need to figure out how to deploy the mitmproxy elsewhere (cloud
run?) before running tests in CI because Github Actions seems to have a
lot of issues with running these docker containers and debugging the
errors has proven quite difficult. While GHA can run docker containers,
mounting a volume with the required certs causes the container to crash.
I'm guessing this is some kind of file permissions issue, but don't have
much visibility for debugging.
## Type of Change
- [x] 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)
- [x] This change requires a documentation update
- [ ] Infrastructure change (CI configs, etc)
- [ ] Non-code change (docs, etc)
- [ ] None of the above: (explain here)
## Test Plan
Run new tests locally with `PINECONE_API_KEY='foo'
PINECONE_INDEX_NAME='bar' poetry run pytest
tests/integration/proxy_config/ -s -v`1 parent a201cb6 commit 6fa0447
File tree
27 files changed
+738
-36
lines changed- .github/workflows
- pinecone
- config
- control
- data
- utils
- tests
- integration/proxy_config
- .mitm
- proxy1
- proxy2
- logs
- unit
27 files changed
+738
-36
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
158 | 158 | | |
159 | 159 | | |
160 | 160 | | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
6 | 54 | | |
7 | 55 | | |
8 | 56 | | |
| |||
27 | 75 | | |
28 | 76 | | |
29 | 77 | | |
30 | | - | |
31 | 78 | | |
32 | 79 | | |
33 | 80 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
154 | 154 | | |
155 | 155 | | |
156 | 156 | | |
| 157 | + | |
| 158 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
81 | | - | |
| 81 | + | |
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
90 | 165 | | |
91 | 166 | | |
92 | 167 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | 3 | | |
5 | 4 | | |
6 | 5 | | |
| |||
10 | 9 | | |
11 | 10 | | |
12 | 11 | | |
13 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
| |||
34 | 36 | | |
35 | 37 | | |
36 | 38 | | |
37 | | - | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
38 | 43 | | |
39 | 44 | | |
40 | 45 | | |
| |||
47 | 52 | | |
48 | 53 | | |
49 | 54 | | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
50 | 61 | | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
56 | 65 | | |
57 | | - | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
6 | 8 | | |
7 | 9 | | |
8 | 10 | | |
| |||
17 | 19 | | |
18 | 20 | | |
19 | 21 | | |
| 22 | + | |
20 | 23 | | |
21 | 24 | | |
22 | 25 | | |
23 | | - | |
24 | 26 | | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
25 | 51 | | |
26 | 52 | | |
27 | 53 | | |
| |||
0 commit comments