-
Notifications
You must be signed in to change notification settings - Fork 35
Getting a request.py error in the code for the "MagTag Lists From Google Spreadsheets" tutorial #103
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
Comments
I tested the code with a dummy URL: https://api.publicapis.org/entries but using the google spreadsheets URL (https://docs.google.com/spreadsheets/d/e/XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/pub?output=tsv) Could it be how the data is being returned? |
Google may have changed up the URLs and something may be getting lost in a redirect. Can you test with curl, something like: curl -iL -A "Adafruit CircuitPython" --http1.1 "https://docs.google.com/spreadsheets/d/e/XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/pub?output=tsv" and see what the sequence of URLs is. edit: found the uRL, and it is getting a |
Looks to me like Requests will do redirects within the same host:
307 to a new subdomain & domain.
|
I kinda follow what you saying .... my initial request is redirected (Temporary redirect 307) to another host but still Google. But does this mean |
I believe |
Okay thanks. Do I have to log a request somewhere to get it fixed? PS. I did try use the redirected URL in but it expires are a certain period :-( |
Yeah, I couldn't get it to work in curl either. A bit unfriendly to require a heavy redirect to access an asset. This issue serves as the request to get the issue addressed, but it is dependent on resources and priorities, or someone motivated enough to fix it sooner ;-). I'm going to also file a new issue in the Guide to keep track there... maybe there's an alternate way to serve up the spreadsheet. |
@wavesailor It might work to get the response headers and parse out the new temporary |
The results I get are definitely a bit strange. I changed the code to retry every minute. This happens if it is successful or if there is a runtime error. It would not work for a few tries, then it would miraculously work, then it would fail again and finally it would crash circuit python. This is the output I got:
|
Now I cannot even get it to run at all :-( I only get this:
So I cannot even try get the redirected URL in because it is failing in the |
Besides the Temporary redirect |
The |
I tried to debug this further but have not had success. |
make sure there is no |
In order to test changes you'll need either A) a build without requests frozen in so that it will use the modified copy in Both options require making a custom build though as far as I know. Option B would then also require making a build for each new change you make to requests which can get tedious. If you're interested in trying it out but haven't already, or don't want to go through the building process perhaps one of us can create a custom build that you can use temporarily for testing this. |
I think putting the library in |
@FoamyGuy Ahhh .... I was starting to pull my hair out or if you have one already then you could just test with this URL and see what is causing it to crash |
Turns out I was missing another (much nicer) option. If you put your modified I probably won't have time to test the URL on a reasonable timeline. Try with the library in the root of the drive. If it still doesn't seem to be using your modified one let me know and I can make you a build without requests frozen in and share that. |
You can also run many different ESP32-S2 UF2s on a MagTag, just find one with the same flash and RAM, maybe the Saola Wrover. The pins won't have nice names, but it will function. |
Okay I got it to read |
You mean
MagTag library is a folder instead of single file, but should work similarly. |
@FoamyGuy Thanks - I'll give that a try. Trying to debug this issue, I find myself a little out of my depth. I can't understand why you would convert a string into bytes and then try convert it into an integer? To me it seems obvious that it will fail
|
The chunk header is a length specified as "hexadecimal number in ASCII": |
@anecdata Thanks for that info. Well I think this would then point to a problem elsewhere - because this is what is in the bytearray: |
It's possible that the chunked
( so I'm not sure how to test that. But assuming that the Just putting in some debug messages into Requests, it appears that the chunk header is broken:
But again, that's more likely a library issue than a Google issue. BTW, Requests does handle the redirect to the alternate host successfully. |
looking at micropython code here https://github.com/micropython/micropython-lib/blob/master/micropython/urllib.urequest/urllib/urequest.py I see the following that redirects not yet supported:
I don't know enough to understand everything but am trying to find the problem. |
Generally not, as-is. The core CircuitPython C code is forked from MicroPython but has some key changes in architecture in parts of the system. Adafruit CircuitPython libraries are often written from scratch, but also modified from MicroPython or other open source libraries. Comments in |
@wavesailor I use different apps for editing and serial console, but I don't think the actual choice of apps matters much, whatever works best for you. |
I think this issues is somehow related: #104 I use his code and it half works now - I don't understand it all |
@wavesailor Glad you're making some progress. I'm working on a pull request for this but have already noticed a mistake in that code - the else block is indented one tab too far. That could be mangling your headers. |
@wavesailor Just finished my pull request. If you're using my code I suggest changing it to:
|
Thanks @MarkTsengTW ... the funny thing is the code original code you put forward works first time around for me but the second time it causes an error. You new updated code causes the same error the stock-standard I hoping that this information may help one of the really clever circuit python guys figure out the issue in |
@wavesailor I'm in stitches! That's hilarious. If it helps, I'm pretty sure the control flow in my original code was handling cookies correctly but deleting all other headers. Perhaps that's a clue to your problem. |
I recently fixed some issues with EDIT: sorry, this fix was for ESP32SPI, not for native ESP32-S2 wifif. |
Hi @dhalbert , I've been trying the latest versions but I'm still running into the same issue.
I downloaded and used these versions - Adafruit CircuitPython 7.3.0 on 2022-05-23; Adafruit MagTag with ESP32S2:
|
Just to be clear, you replaced all copies of |
I've been looking into this issue too and I've been able to reproduce it on MagTag with CP 7.3.0 and on Blinka 8.0.2 both with adafruit_requests.py 1.12.0. I've found a code change that appears to make it work (at least it doesn't error out) but I wasn't sure I found the real root cause. So I set up a unit test in Adafruit_CircuitPython_Requests that mocks the call to Google servers and step through the code. But it doesn't seem to reproduce in a unit test (yet.) Dan, do you have a commit id for the change to |
Sorry, the fix I mentioned above was for ESP32SPI (AirLift), not for native ESP32-S2 wifi. I have been looking at several wifi issues and got them confused. The ESP32SPI error caused a chunking error in What is the code change you made to make it work? |
This change seems to work OK but I haven't done much testing and I'm not sure why Note this is in diff --git a/adafruit_requests.py b/adafruit_requests.py
index 2225ca9..213be28 100644
--- a/adafruit_requests.py
+++ b/adafruit_requests.py
@@ -332,13 +332,9 @@ class Response:
if self._remaining and self._remaining > 0:
self._throw_away(self._remaining)
elif self._chunked:
- while True:
- chunk_header = bytes(self._readto(b"\r\n")).split(b";", 1)[0]
- chunk_size = int(bytes(chunk_header), 16)
- if chunk_size == 0:
- break
- self._throw_away(chunk_size + 2)
- self._parse_headers()
+ # read the remaining response chunks into a small temporary buffer and discard
+ buf = bytearray(32)
+ while self._readinto(buf) != 0: pass
if self._session:
self._session._free_socket(self.socket) # pylint: disable=protected-access |
Proposed fix for issue #103. Fixes an issue with _throw_away() and large chunked redirects.
I'm still getting an error with regards to this issue
I'm using these versions of circuit python and libraries:
This is what the MagTag files look like
If it helps, this is the URL I'm having issues with: |
@klocs Do you have any idea why your fix might not be working for @wavesailor? |
Let me take a look. |
@wavesailor @dhalbert I'm pretty sure the problem is that 7.3.1 has the older version of Until there is a new release of CP for MagTag, you can copy the I tested it and it works for me™. 😉 |
@wavesailor You can see what modules are included by typing |
Ahh, thanks! I'd forgotten that >>> import sys
>>> sys.path
['', '/', '.frozen', '/lib'] So a frozen module overrides what is in |
I'm getting an error running the following code: https://github.com/adafruit/Adafruit_Learning_System_Guides/blob/main/MagTag_Google_Sheets/weekly_planner/code.py
I use
wget
on my Pi to retrieve my URL just fine, but it does not work on the MagTagI'm using the latest libraries and Circuit Python for MagTag
The text was updated successfully, but these errors were encountered: