search

CartoDB / carto-python

CARTO Python client
https://carto.com
BSD 3-Clause "New" or "Revised" License
154 stars 62 forks source link

client request arg is overwritten if already set #97

Closed andy-esch closed 5 years ago

andy-esch commented 5 years ago

Since https://github.com/CartoDB/carto-python/pull/90, cartoframes client param is overwritten. In cartoframes we set it here for use in all major cartoframes methods.

I don't know the best course of action for resolving this situation, but I'm happy to help because we are no longer able to track cartoframes usage.

cc @alrocar

alrocar commented 5 years ago

Hey @andy-esch , thanks for reporting. I wasn't aware of this change and I'm OOO until next Monday I can take a deeper look by then, but in the meanwhile I'm suggesting you to try something.

Would it work to subclass the _ClientIdentifier class in cartoframes, so that you return anything you want for cartoframes in get_client_identifier (and maybe get_user_agent as well) and then use that subclass in your APIKeyAuthClient instance?

andy-esch commented 5 years ago

@alrocar thanks for the response. I'm OOO until Monday too and will take a look then. Interesting idea on subclassing _ClientIdentifier.

andy-esch commented 5 years ago

Hey @alrocar, I thought more about subclassing _ClientIdentifier and feel uncomfortable about using a private class that could change without warning. Do you have any other suggestions about how we could change the Python SDK source instead? Maybe a check if a client param is already set?

alrocar commented 5 years ago

Oh I see your point. Checking if the client param is present looks good to me then :+1:

I can try to do it this week, but feel free to send a PR if you want.

Thanks!!

andy-esch commented 5 years ago

PR here: https://github.com/CartoDB/carto-python/pull/99

andy-esch commented 5 years ago

solved in #97