bug in retry code for Star Fragment Lamp

[jdimpson]

I believe there is a bug in the retry code used get_request() in the Star Fragment Lamp code.

The retry code at line 65 (

Adafruit_Learning_System_Guides/Star_Fragment_Lamp/code.py

Line 65 in a2a2916

n = ping

) is:
n = ping
This simply assigns the value of ping to the variable n. It does not invoke ping. It should be:
n = ping()

The reason the code appears to work is that the retry code is called like this (on line 78 and at least one other location):
now = get_request(5, io.receive_time())

Specifically, io.recieve_time() is being invoked before get_request(). The result of io.recieve_time() is then being passed into get_request() as the value for ping, which as shown above simply copies that value into n, which it then returns as the result of get_request(). So io.recieve_time() is being invoked outside of the retry code in get_request(), which mean it doesn't benefit from any retries. Instead, get_request() should be invoked like this:
now = get_request(5, io.receive_time)

This passes a reference to the io.recieve_time() as the value of ping, which with the change I suggest above, will then be invoked inside the retry loop in get_request.

(Edit: formatting typo)

[BlitzCityDIY]

hello- "ping" is the name of the variable here (but perhaps i should change it to avoid confusion). in the function, the request is passed inside the try/except loop, which is nested in the for loop. so it will try making the request the # of times defined as tries. n is the result of the request, in this case either JSON or struct_time. it did retry in my testing.

i did test your suggested fix and it errors out with 'Response' object is not callable and AttributeError: 'bound_method' object has no attribute 'tm_mday'.

[jdimpson]

The line where get_request(5, io.receive_time()) is being invoked are first invoking io.recieve_time() and sending the resulting time structure as the second argument of get_request(). The second argument, ping then contains the result of io.recieve_time() through out. So it won't ever trigger an exception because all that's happening is the time structure in ping is being assigned to the variablen (unless something went so wrong that assignment fails).

You're seeing the 'Response' object is not callable error because my bug report didn't include the fix for line 87, whih looks like this:
response = get_request(5, requests.get(weather_url))

With the proposed change to get_requests() that activates retries, line 87 needs to change like this:
response = get_request(5, lambda : requests.get(weather_url))

I didn't realize this line was present until I submitted my pull request, which is why I didn't mention it in the bug report.

The lambda expression lets you delay the invocation of requests.get(weather_url) so that it also runs in the retry loop. It's needed because we can't send the bare reference to requests.get like we did for io.recieve_time, because the former needs an argument passed to it. Using the lambda expression lets us embed the argument into a new piece of code that will then be run in a retry loop.

This kind of thing is hard to test because it would require causing io.recieve_time() to fail the first time it is invoked but not the second time.

[jdimpson]

This might help for anyone who needs a refresher for passing functions as parameters/ variables in python.