-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update the Google Sheets API calls to v4 #223
base: master
Are you sure you want to change the base?
Conversation
I tweaked my key somewhat to restrict it to |
I had restricted the key to HTTP requests only on Should work now from anywhere, but still restricted to the Google Sheets API |
What is the environment you're using to build this project? I'd love to test this branch but it feels like there is a bit of a dependency / node issue going on (currently encounting issue with building node-gyp). |
Nothing special near as I can tell.
I didn't do anything more than that and I haven't done any deployment, all local host. I can give you anything else you need/want |
A couple of other things I noticed on my own code:
|
This PR updates the API calls to Google Sheets to use v4 instead of v3 which was shut down August 2. Fixes Asmor#217, Fixes Asmor#218, and Fixes Asmor#222 with some caveats, see below. Full disclosure: I have no idea what I'm doing really with JS, so there are likely to be improvements to be made. Potentially Outstanding Issues: * Uses my personal API Key. Its restricted to Google Sheets API only, but I would prefer it be someone else's. Not really a problem right now. * Each Sheet is loaded in full through a GET request, returned entierly as JSON. This results in the load being a bit slow (and a fair amount of data comes down, the Official is 33MB of plain text returned for example. The load only happens once per sheet. * I left in all the old code where I could in case improvements can be made. * I do not know JS, so any improvements, especially in the Promises area are likely very welcome. Removed some debug lines I left in sending to console.
Removed the debug lines I had left in, squashed the commit into the main one and force pushed. Sorry to those who had checked this out to test, however, there should be no functional difference for debugging issues. |
What version of node is installed by brew? |
Node version 10.15.3. I haven't done a clean install in a while, so its whatever is on my machine here. I have tested this on a fresh ubuntu machine where I did an |
10.15.3 works; 12.x and 16.x don't work (node-sass isn't compatible). |
The |
I don't want to update the Node/node-sass in this PR anyways. I want this PR to do the bare minimum to get KFC working again without a huge security hole to maximize the chance it gets merged and deployed. @Asmor did say that PRs won't help (at least if its a rate limiting problem), so I wanted to make this as clean and straightforward a swap of the v3 -> v4 Sheets API that I could just in case it was a simple enough merge/deploy to keep the site going even a little longer. |
Looking at modifying packages (and potentially needing to update for API changes and what not) should not be in scope for this delta. |
I fully agree. Just making an admittedly needless/confusing observation, apologies. |
hi all |
This PR updates the API calls to Google Sheets to use v4 instead of v3
which was shut down August 2.
Fixes #217, Fixes #218, and Fixes #222 with some caveats, see below.
Full disclosure: I have no idea what I'm doing really with JS, so there
are likely to be improvements to be made.
Potentially Outstanding Issues:
but I would prefer it be someone else's. Not really a problem right now.
as JSON. This results in the load being a bit slow (and a fair amount
of data comes down, the Official is 33MB of plain text returned for
example. The load only happens once per sheet.
made.
area are likely very welcome.
Edits by maintainers are on and I'm happy to make any edits, but without explicit "do exactly this" I'll be fumbling around.
Edit: Forgot to add the time stamp note, since added