Add support for static deletion

[ob-stripe]

r? @brandur-stripe @remi-stripe

A very hacky and experimental way of adding support for static deletion on deletable resource classes.

Normally, Python doesn't allow you to have both a class and instance method with the same name. This PR works around this limitation by defining a special class_or_instance_method decorator. The decorator is defined as a non-data descriptor. Descriptors receive both the instance and the class as arguments in their __get__ method. The method checks if the instance variable (obj) is None or not to decide whether the method should be called as a class method (with the type variable objtype as the first argument) or as an instance method (with the instance variable obj as the first argument).

The actual "class or instance method" can then check if its first argument is a type or not to decide whether it was called from a class or from an instance and behave appropriately.

Tagged as WIP because this is rather hacky and I want to get your thoughts on it first. If we're comfortable with this approach, I'll add some safeguards around arguments (right now if you call e.g. stripe.Customer.delete() without any arguments, you get a very unclear "tuple index out of range" error because the code tries to access args[0] to get the ID) and write additional tests.

[remi-stripe]

Damn, I said it could not be done in Python and obviously you immediately show it can. This is really cool but I have no idea how safe it is to take that approach, especially as we'd have to do this for all the instance methods in the lib (though there are not that many beyond retrieve).

How would it work for nested resources by the way? In the sense of trying to have delete static on Person, versus having to do Account.deletePerson() be static (though that could work too).

I'll defer to @brandur-stripe on whether this is the wrong approach versus just doing a new major revision that only has static methods.

[ob-stripe]

we'd have to do this for all the instance methods in the lib (though there are not that many beyond retrieve).

retrieve is already static ;) and we already have modify for update requests, so I think that leaves only delete and all non-CRUDL instance methods (capture, etc.).

How would it work for nested resources by the way? In the sense of trying to have delete static on Person, versus having to do Account.deletePerson() be static (though that could work too).

Classes for parent resources already have class methods for all nested resources operations, e.g. Account.delete_person(acct_id, person_id). I think it's better this way (in the sense that removing a person from an account is more of an "account operation" than a "person operation"), and it also sidesteps the issue of nested resources with multiple possible parent resources (cards, bank accounts).

[brandur-stripe]

Wow OB, this is some serious Gandalf-level Python wizardry. Nice patch!

So I would probably have said that my first preference would have been to come up with two slightly different method names for the static versus non-static action even if the difference was somewhat arbitrary (destroy versus delete). This seems less likely to cause confusion to me.

That said, I appreciate that "delete" is probably the right word from an API standpoint (as it meshes well with DELETE), and we already have static "delete" functions in languages like Go, so I don't feel too strongly about it. This seems fine to me if you want to go with it.

[ob-stripe]

I've updated the PR with a slightly more complicated decorator/descriptor (a desecrator? 😛) but I think the result is really nice.

The new decorator is applied directly on a regular instance method, and takes an alternate class method name as its parameter. When the method is called on the class, then the alternate class method is called, otherwise the regular instance method is called.

ptal @brandur-stripe @remi-stripe

[brandur-stripe]

Sorry, approved a little prematurely, but this approach seems okay to me.

[ob-stripe]

NP, thanks Brandur! We're just going to add some more tests before merging.

[remi-stripe]

@ob-stripe I just did all the examples, PTAL

[ob-stripe]

LGTM

[remi-stripe]

@ob-stripe I force pushed as I had forgotten to do Terminal. Please make sure I did not miss any other

[ob-stripe]

Released as 2.24.0.