client-python icon indicating copy to clipboard operation
client-python copied to clipboard

RESTClient handles base parameter incorrectly

Open stepancheg opened this issue 7 months ago • 1 comments

With code like this:

RESTClient(base = "http://my-local-polygon-proxy")

streaming API does not work correctly.

On this line:

https://github.com/polygon-io/client-python/blob/f789d2905bf9410ce372ff8c3dd8549534c4603d/polygon/rest/base.py#L231

Next URL tries to replace in returneded URL, that is https://api.polygon.com the substring BASE that is http://my-local-polygon-proxy, and it results in incorrect URL.

Instead, this code should just scheme and hostname.

stepancheg avatar Jun 06 '25 18:06 stepancheg

Hello, just a heads up I'm looking at this now.

justinpolygon avatar Jun 10 '25 22:06 justinpolygon

Hi @stepancheg, here's a draft PR that likely solves your request. This was a new feature in that base was mostly used for internal testing against development endpoints and we never really thought about people using this as a proxy address. Do you mind checking out https://github.com/polygon-io/client-python/pull/890 and seeing if this makes sense? Sorry for the delay here it took a bit to understand what was actually needed.

justinpolygon avatar Jun 23 '25 07:06 justinpolygon

@justinpolygon

client = RESTClient(
    api_key="your_api_key",
    base="https://api.polygon.io",  # API endpoint
    proxy="http://proxy.example.com:8080",  # Proxy server
    trace=True
)

This is not how we intent to use the proxy —

  • in our case proxy provides authentication (API key), so our clients would not have access to API key
  • additionally, custom server URL can be used to mock API server (in tests, for example)

"Proxy" parameter can be used as workaround, but that is not the most straighforward fix for the problem.

The problem I pointed to, still exists. There's a bug on this line of code:

https://github.com/polygon-io/client-python/blob/f789d2905bf9410ce372ff8c3dd8549534c4603d/polygon/rest/base.py#L231

This PR attempts to fix it

        # Check if path is a full URL or a relative path
        if path.startswith("http"):
            url = path
        else:
            url = self.BASE + path

But it does not: if path does not start with BASE, this won't prepend the BASE.

This code could just:

  • remove schema/hostname from path
  • prepend self.BASE

That said, explicit support of HTTP proxy is a good thing to do.

stepancheg avatar Jun 23 '25 13:06 stepancheg

Hey @stepancheg, are you working with https://github.com/polygon-io/client-python/issues/882? Since these both came in around the same time I just wanted to see if this was the same use-case. Are you able to support https via your internal service that adds the api key? If so, it should just work without any code changes. It's the http/https issue that seems to be the source of the issue.

justinpolygon avatar Jun 23 '25 15:06 justinpolygon

@justinpolygon yes, we are the same project.

Our internal internal proxy is http not https service.

stepancheg avatar Jun 23 '25 16:06 stepancheg

Great, thanks @stepancheg. Have you tested a solution on your end by patching the client? I'm just wondering how I could actually test this patch on my end. I guessing you're using this proxy like a secret manager or something and it would be helpful to know if a fix actually works without releasing a client.

Also, how are you dealing with auth in the client before the proxy? Right now, we check to see if there is either an API passed in or an env var. So, you would likely see an error when you tried to make a request without auth. If you're using the proxy are you stripping the auth out somehow?

I have a potential fix but I just want to verify this would work and have some way of testing.

justinpolygon avatar Jun 23 '25 17:06 justinpolygon

Sorry, one more thing @stepancheg. I'd be happy to chat via a call if you're around. Going back and for on github for something like this can be time consuming. I think you folks have an open chat with Jack right? He can set something up if you think it's needed.

justinpolygon avatar Jun 23 '25 17:06 justinpolygon

@justinpolygon Hi Justin, I don't have bandwidth to help test your changes unfortunately. But I have a commit that fixes the problem for us. I believe you can compare the output of my commit with respect to your code changes to verify. https://github.com/yiweny/client-python/commit/055aeb7a1f7db53c91e80decd4628fa2872d9399#diff-78e47ba54099d2a8d6d1ea2d5b86d0ee312951e1ccee021a07855b10b01ec97fR213-R239 Thanks a lot for helping us!

yiweny avatar Jun 25 '25 04:06 yiweny

Perfect, thank you! Checking it out now.

justinpolygon avatar Jun 25 '25 16:06 justinpolygon

Hey @stepancheg, this should be fixed now in the v1.15.0 release. I implemented the fix from https://github.com/yiweny/client-python/commit/055aeb7a1f7db53c91e80decd4628fa2872d9399#diff-78e47ba54099d2a8d6d1ea2d5b86d0ee312951e1ccee021a07855b10b01ec97fR213-R239. You'll just need to run pip install -U polygon-api-client to get the latest release.

justinpolygon avatar Jul 07 '25 08:07 justinpolygon