Propagate proxies and ssl_verification args to cloud services#2607
Propagate proxies and ssl_verification args to cloud services#2607diego-plan9 merged 28 commits intoQiskit:mainfrom
Conversation
diego-plan9
left a comment
There was a problem hiding this comment.
Thanks @alodolo ! As #2593 might require some time, it makes sense to move this PR forward with the goal of closing the issue.
And thanks for bringing the proxy testing situation into a better state as part of this PR - actually, this initial review is focused on mitmproxy usage and the infra around it, preparing for next steps.
|
Hi @diego-plan9 , I reverted all the changes link to the switch to Right now in the PR I left the patch in the source code with proxy and ssl_verify parameters injections if the fixe need to be merged without changing the integration tests. |
diego-plan9
left a comment
There was a problem hiding this comment.
Thanks @alodolo !
I reverted all the changes link to the switch to
mitmproxyand the proxies integration test and I will create an another dedicated issue & PR. It seems thatpproxyhas compatibility issues with the latest version of python andmitmproxyhas issues withpython 3.10.
Thanks for looking into it - perhaps a less recent mitmproxy version might fit the bill when it comes to supporting the right Python versions, as it would be great to replace pproxy eventually. Indeed, the goal is getting the neat fix in place: looking forward for the follow-up issue and PR, with lower priority.
Other than some nits, it is just one test away from being ready! As another small nit: can you rename the PR to something a bit more human-readable, so it can be easier to parse when going through history?
Co-authored-by: Diego M. Rodríguez <diego.plan9@gmail.com>
…mits" This reverts commit ef4108b.
e122cbf to
50e1ed0
Compare
| "urllib3>=2.4.0", | ||
| "python-dateutil>=2.9.0", | ||
| "ibm-platform-services>=0.55.3", | ||
| "ibm-platform-services>=0.61.1", |
There was a problem hiding this comment.
Thanks - for future reference, this is due to GlobalCatalogV1.get_catalog_entry() and other methods not propagating the kwargs until https://github.com/IBM/platform-services-python-sdk/pull/286/changes#diff-8c42907892d2dd2139f88503f2d0049bcec384c7def602131f1a46ba3e750995L292-R316 landed.
Summary
This pull request should fix the following issues: #2382 and #2592.
It injects ssl_verification and proxy arguments to outgoing requests.
It also patches the proxy integration test.Details and comments
Remarques:
Proxy and ssl_verification injection
The requests parameter injection for
GlobalSearchV2andGlobalCatalogV1is done during the search method. The PR host verification disable now works #2593 is adding ssl_verification at the service instanciation level, however those services don't have (yet) the constructors to disablessl_verification.IAM service
It seems that the class that handle the IAM service is instanciated multiple times in different ways (
IAMAuthenticatorinCloudAccount, andIAMTokenManagerinCloudAuth). I would recommend some refactoring to use only 1 configured IAM service in the code.Integration testing - proxyIt seems that integration tests for the proxy were skipped because it was conditioned only foribm_cloudservices.On top of that the
pproxylib have bugs with the latest versions of python and is not maintained anymore (Fixed no current event loop error when using current uvloop version qwj/python-proxy#202). I switched the lib formitmproxy(MIT license, python API, 40k stars and up to date - https://github.com/mitmproxy/mitmproxy).Fixes #
integration tests for proxy