Improvement: Allow users to provide a custom credentials file to oauth()

[slmtpz]

closes #889

[slmtpz]

One thing that bugs me a little is that the filename inputs of load_credentials and store_credentials have a default value in the function definition even though the default filename values are passed within oauth to these functions.
I introduced a new parameter filename for the flow functions, namely local_server_flow and console_flow. It doesn't have a default value in the function definitions as there is no need.

For consistency sake;
1- Should I drop the default values of filename from load_credentials and store_credentials? Are these considered a part of gspread API, would this be a breaking change?
2- Or, should I add a default value to filename in the flow functions?
3- Or, is this already good as is?

[lavigne958]

One thing that bugs me a little is that the filename inputs of load_credentials and store_credentials have a default value in the function definition even though the default filename values are passed within oauth to these functions.
I introduced a new parameter filename for the flow functions, namely local_server_flow and console_flow. It doesn't have a default value in the function definitions as there is no need.

For consistency sake;
1- Should I drop the default values of filename from load_credentials and store_credentials? Are these considered a part of gspread API, would this be a breaking change?
2- Or, should I add a default value to filename in the flow functions?
3- Or, is this already good as is?

Hi @slmtpz, thank you very much for your contribution.

about the default values in load_credentials and store_credentials they are here in case someone want to write its own oauth scenario and use these functions, this way they are covered with the right default values.

1. to me, yes it could be braking changes, if someone has its own oauth function (for some reason, like doing thing in-between load, flow, store) then they assume the default value and use it this way. removing it will brake this code.
2. Again, to me, you should. this is open for discussion though. Like the above example it is the same reason. On the top of that before you PR anyone using GSpread used the default store location for these credentials by using the function console_flow or local_flow now it should work the same way, so the filename should have a default value again here using DEFAULT_CREDENTIALS_FILENAME
3. unfortunately not 🙂

Hope this answer your questions and help you move forward 😉

Apart from this, your PR is very welcome, I will run the test suite very soon on it.

[slmtpz]

One thing that bugs me a little is that the filename inputs of load_credentials and store_credentials have a default value in the function definition even though the default filename values are passed within oauth to these functions.
I introduced a new parameter filename for the flow functions, namely local_server_flow and console_flow. It doesn't have a default value in the function definitions as there is no need.
For consistency sake;
1- Should I drop the default values of filename from load_credentials and store_credentials? Are these considered a part of gspread API, would this be a breaking change?
2- Or, should I add a default value to filename in the flow functions?
3- Or, is this already good as is?

Hi @slmtpz, thank you very much for your contribution.

about the default values in load_credentials and store_credentials they are here in case someone want to write its own oauth scenario and use these functions, this way they are covered with the right default values.

1. to me, yes it could be braking changes, if someone has its own oauth function (for some reason, like doing thing in-between load, flow, store) then they assume the default value and use it this way. removing it will brake this code.
2. Again, to me, you should. this is open for discussion though. Like the above example it is the same reason. On the top of that before you PR anyone using GSpread used the default store location for these credentials by using the function console_flow or local_flow now it should work the same way, so the filename should have a default value again here using DEFAULT_CREDENTIALS_FILENAME
3. unfortunately not 🙂

Hope this answer your questions and help you move forward 😉

Apart from this, your PR is very welcome, I will run the test suite very soon on it.

Thanks for your elaborate answer @lavigne958 ! I just updated the function definitions so that filenames have default value for the backward compatibility.