femueller / python-n26

💵 Unofficial Python client for n26 (Number 26) - https://n26.com/
MIT License
195 stars 34 forks source link

Implementing `get_token` #108

Closed cimentadaj closed 3 years ago

cimentadaj commented 3 years ago

I've been meaning to contribute to python-n26 since I use it so much and I was trying to understand how the source code works. I noticed this and was wondering if you would be interested in discussing a possible implementation and considering a PR. One thing that I'm curious is whether the duplication code @get_token merits the decorator (this decorator would be used essentially in all get_* functions, a lot of repetition) since _do_request already takes care of this in a single line. Did you have in mind any specific benefit?

My idea was to redefine get_token as a decorator and just save the token in token_data and _do_request can just grab that from self.

Thanks for your open source work, it's great!

markusressel commented 3 years ago

Happy to hear python-n26 is useful to you 🚀

Not sure about the usefulness of this decorator... the comment you linked is about 4 years old, and there hasn't really been the need for it. I think this might even be a remnant from the times before the "rewrite" from my end, which already drastically reduced the amount of duplicated code. Not very fond of adding back one line per function 😄

The one thing I wanted to improve quite a while ago was the usability of this project as a library within other projects, specifically the way that other developers can integrate user input during authentication. This is even an issue for the CLI, which is why this hack exists. The design around this simply hasn't matured much since it wasn't requested. Since N26 has made it so difficult to talk with their API without reauthenticating constantly, not many other projects using python-n26 have popped up (afaik), so there really isn't the need for any change right now. Also the project generally seems to be quite stable, with very few issues getting opened per year.

I very much appreciate your train of thought, but I am not sure where a contribution would be needed right now.

cimentadaj commented 3 years ago

Thanks for the quick response. Yep, your train of thought is spot on. I'll close this and open a new one if an idea comes up. Thanks @markusressel !