search

civisanalytics / civis-python

Civis API Python Client
BSD 3-Clause "New" or "Revised" License
34 stars 26 forks source link

Add `ignore_deprecated` flag to `APIClient` #345

Closed elsander closed 4 years ago

elsander commented 4 years ago

The API client currently ignores any endpoints with a deprecation warning. This means that endpoints effectively cannot be deprecated-- once they are tagged deprecated, they are inaccessible from the client.

One option would be to automatically pull in these endpoints, which would allow for deprecation, but would make deprecated endpoints visible to new users, which may not be desirable.

The alternative I'd propose is to add an ignore_deprecated flag when instantiating the client, which defaults to True. This means that existing code based on newly deprecated endpoints would still break, but could be updated to work until the endpoint is removed entirely.

elsander commented 4 years ago

H/t @mheilman for raising this issue

elsander commented 4 years ago

Update: an API user's expectation would be that deprecated endpoints should appear in the client. So rather than ignore_deprecated, we can have an include_deprecated flag which defaults to True. Users who want the old behavior can set it to False. Not sure how often that flag will be used in practice, but in theory if you're building a new tool, you wouldn't want to write anything that relies on deprecated endpoints, so you might rather not have them junking up the client spec. To be honest, I'd also be comfortable just changing this default to always include deprecated endpoints, rather than add a new parameter.

salilgupta1 commented 4 years ago

I'm pretty sure no one is going to know to use this flag. Can we always include deprecated endpoints in the client? When we remove the endpoints we'll have to do some sort of change management via our Client success teams (what we do right now).

elsander commented 4 years ago

Yeah, I think I've been convinced that it's better to just always include deprecated endpoints (as long as they have some indication to the user that they're deprecated).