gspread icon indicating copy to clipboard operation
gspread copied to clipboard

Refactor to remove old auth.console_flow references

Open mafrosis opened this issue 1 year ago • 7 comments

mafrosis avatar Jul 19 '24 00:07 mafrosis

thanks for the suggestion!

What motivates this change? You remove a keyword argument to some functions. Some people could rely on using these, no?

alifeee avatar Jul 23 '24 14:07 alifeee

thanks for the suggestion!

What motivates this change? You remove a keyword argument to some functions. Some people could rely on using these, no?

no people can't rely on them as the flow is not working anymore with Google Authentication system.

Thank you for this clean, I forgot about it.

so far no one should be able to use this flow for months, though we should probably keep it for a major release just in case ?

lavigne958 avatar Jul 23 '24 20:07 lavigne958

Hey, sorry for the lack of description 😄

For future readers: https://github.com/googleapis/google-auth-library-python-oauthlib/commit/1fb16be1bad9050ee29293541be44e41e82defd7

I was working on some cross-library compatibility regarding Google API auth flows and tokens and such (gspread & kuzmoyev/google-calendar-simple-api & jeremyephron/simplegmail), and I noticed all this effectively dead code in the gspread auth library. Just doing the good citizen open source thing and sending out a PR for y'all

Thanks for gspread, it's awesome 🫡

mafrosis avatar Jul 24 '24 04:07 mafrosis

thanks for this clean-up, effectively it's dead code, though I'd rather merge it in the next major release just in case, we've seen many issues like this before where it should be ok to merge this and it was not for a few users.

We'll keep this PR open for now until we make next major release and then merge it.

thank in advance for this contribution :upside_down_face:

lavigne958 avatar Jul 31 '24 16:07 lavigne958

Ping on this PR, I just spent a morning trying to use the documented console_flow before realizing that it's non-functional.

@mafrosis / @lavigne958

Chris113113 avatar Jul 01 '25 18:07 Chris113113

Looks like this missed the 6.2 release - @lavigne958, I'll rebase this shortly and update the PR. Maybe can include in 6.3?

mafrosis avatar Jul 01 '25 20:07 mafrosis

@burnash @lavigne958 PR updated, please merge

mafrosis avatar Aug 06 '25 02:08 mafrosis