Better exception for Source.detach() on detached sources

[jleclanche]

stripe-python/stripe/api_resources/source.py

Line 29 in 8629450

raise NotImplementedError(

Could this get its own exception type? I want to give customers the option to remove a source from their payment methods, and right now I have to do something like this:

try:
    source.detach()
except NotImplementedError:
    resync(source)

Catching notimpl is weird, and this isn't really the appropriate exception type I think anyway.

[ob-stripe]

Hey @jleclanche, thanks for the contribution!

Even if the exception type could be improved, I'm a bit reluctant to pull this in as it's a breaking change.

IMHO, the fact that you don't know whether a source is attached to a customer or not before calling detach() is indicative of an issue in your data model. This exception is there to point out a programming error and should not be used in regular control flows.

That said, I'll leave this open for now and ask for second opinions.

[jleclanche]

@ob-stripe you can have the exception subclass NotImplementedError if you want it to be backwards-compatible.

IMHO, the fact that you don't know whether a source is attached to a customer or not before calling detach() is indicative of an issue in your data model.

I do know, but it's completely possible for data to go out of sync (eg. a second client, such as a dashboard user, can detach the source, webhooks might not trigger in time etc).

[ob-stripe]

you can have the exception subclass NotImplementedError if you want it to be backwards-compatible.

I guess. Seems a bit hacky though!

it's completely possible for data to go out of sync (eg. a second client, such as a dashboard user, can detach the source, webhooks might not trigger in time etc).

Point taken.

We don't have firm plans yet, but there are a few backwards-incompatible changes I'd like to ship mid term. I've added your request to the list.

Tagging this as future for now.