Skip to content
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

add context manager vs explicit close() example #185

Closed
wants to merge 5 commits into from

Conversation

DJDevon3
Copy link
Contributor

This should be handy to point beginners at for recommended syntax to use with API requests. This was discussed in this weeks meeting. Before I proceed to update all examples to use with I'd like to add an example that covers why it's a better way to go. Instead of explaining to a beginner, can point them at this script so they can run it and see the code comments for themselves that details the process. If you have any wording or syntax changes to improve it feedback always appreciated!

This should be handy to point beginners at for recommended syntax to use with API requests.
@anecdata
Copy link
Member

Also, comments about closing the socket should be about closing the response. Socket-level details are handled invisibly within requests.

@DJDevon3
Copy link
Contributor Author

@anecdata Thank you. I was confused about how to word that. Will change.

@justmobilize
Copy link
Collaborator

Would it be worth logging the connections that ConnectionManager is tracking to show it's working?

@DJDevon3
Copy link
Contributor Author

@justmobilize I don't think so since that's not something that's been done previously and serial prints can get pretty lengthy with API examples as it is. Keeping it behind the abstraction is fine but if you want to add a way to enable a debug=True for those who want a more verbose output that would be nice.

@justmobilize
Copy link
Collaborator

Your call.

It would just to be the count, so just one other line (well for each one). there's already 22 print statements in there. Could replace the content_type statement, since it's not related to this example.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@justmobilize The content-type is just an example that shows that you can still use the response data after the response is closed for both methods. Perhaps I should change it to something more obvious like an actual endpoint?

Sounds like I misunderstood your suggestion. Can you elaborate with a snippet? I thought you meant printing everything Connection Manager is doing under the hood which would be a substantial example... and one that might be cool to see as a separate example in itself.

@DJDevon3
Copy link
Contributor Author

There are no actual endpoints only headers in this example. Would add more complexity to add endpoints into the example.

@justmobilize
Copy link
Collaborator

Something like:

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
print(f"Current used sockets: {used_sockets}")

I don't have a device handy (on spring break) so can't test...

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@justmobilize I sprinkled that throughout and it always comes up with Current used sockets: 0

Current used sockets: 0

Connecting to WiFi...
Signal Strength: -57
✅ Wifi!

Current used sockets: 0
----------------------------------------
Explicit Close Example
Current used sockets: 0
 | 🆗 Status Code: 200
 |  | Response Timestamp: Thu, 11 Apr 2024 16:36:14 GMT
 | ✂️ Disconnected from https://httpbin.org/get
Current used sockets: 0
 |  Content-Type: application/json

versus

----------------------------------------
Context Manager WITH Example
Current used sockets: 0
 | 🆗 Status Code: 200
 |  | Response Timestamp: Thu, 11 Apr 2024 16:36:33 GMT
 | ✂️ Disconnected from https://httpbin.org/get
Current used sockets: 0
 |  Content-Type: application/json

Both examples are functionally identical
However, a with statement is more robust against disconnections mid-request and automatically closes the response.
Using with statements for requests is recommended

@justmobilize
Copy link
Collaborator

response = requests.get(JSON_GET_URL)
json_data = response.json()
if response.status_code == 200:
    print(f" | 🆗 Status Code: {response.status_code}")
else:
    print(f" |  | Status Code: {response.status_code}")
headers = json_data["headers"]
date = response.headers.get("date", "")
print(f" |  | Response Timestamp: {date}")

################ ADD ONE HERE, BEFORE THE CLOSE ################

# Close response manually (prone to mid-disconnect socket errors, out of retries)
response.close()

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@justmobilize I put them everywhere and no difference.

# SPDX-FileCopyrightText: 2024 DJDevon3
# SPDX-License-Identifier: MIT
# Updated for Circuit Python 9.0
""" WiFi Context Manager Basics Example """

import os

import adafruit_connection_manager
import wifi

import adafruit_requests

# Get WiFi details, ensure these are setup in settings.toml
ssid = os.getenv("CIRCUITPY_WIFI_SSID")
password = os.getenv("CIRCUITPY_WIFI_PASSWORD")

# Initalize Wifi, Socket Pool, Request Session
pool = adafruit_connection_manager.get_radio_socketpool(wifi.radio)
ssl_context = adafruit_connection_manager.get_radio_ssl_context(wifi.radio)
requests = adafruit_requests.Session(pool, ssl_context)
rssi = wifi.radio.ap_info.rssi

JSON_GET_URL = "https://httpbin.org/get"
used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print(f"Current used sockets: {used_sockets}")
print(f"\nConnecting to {ssid}...")
print(f"Signal Strength: {rssi}")
try:
    # Connect to the Wi-Fi network
    wifi.radio.connect(ssid, password)
except OSError as e:
    print(f"❌ OSError: {e}")
print("✅ Wifi!\n")

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print(f"Current used sockets: {used_sockets}")

print("-" * 40)
# This method requires an explicit close
print("Explicit Close Example")
response = requests.get(JSON_GET_URL)
json_data = response.json()

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print(f"Current used sockets: {used_sockets}")
if response.status_code == 200:
    print(f" | 🆗 Status Code: {response.status_code}")
else:
    print(f" |  | Status Code: {response.status_code}")
headers = json_data["headers"]
date = response.headers.get("date", "")
print(f" |  | Response Timestamp: {date}")

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

# Close response manually (prone to mid-disconnect socket errors, out of retries)
response.close()
print(f" | ✂️ Disconnected from {JSON_GET_URL}")

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print(f"Current used sockets: {used_sockets}")
# This example shows json_data still available after response close
content_type = response.headers.get("content-type", "")
print(f" |  Content-Type: {content_type}")

print("\nversus\n")

print("-" * 40)
# Closing response is included automatically using "with"
print("Context Manager WITH Example")
response = requests.get(JSON_GET_URL)
# Wrap a request using a with statement
with requests.get(JSON_GET_URL) as response:
    date = response.headers.get("date", "")
    json_data = response.json()
    
    used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
    
    print(f"Current used sockets: {used_sockets}")
    if response.status_code == 200:
        print(f" | 🆗 Status Code: {response.status_code}")
    else:
        print(f" |  | Status Code: {response.status_code}")
    headers = json_data["headers"]
    print(f" |  | Response Timestamp: {date}")

# Notice there is no response.close() here
# It's handled automatically in a with statement
# This is the better way.
print(f" | ✂️ Disconnected from {JSON_GET_URL}")

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print(f"Current used sockets: {used_sockets}")
# This example shows json_data still available outside of with.
content_type = response.headers.get("content-type", "")
print(f" |  Content-Type: {content_type}")

used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])

print("\nBoth examples are functionally identical")
print(
    "However, a with statement is more robust against disconnections mid-request "
    + "and automatically closes the response."
)
print("Using with statements for requests is recommended\n\n")

removed and put only where suggested. no difference. 0 sockets.

@justmobilize
Copy link
Collaborator

Oh, this example isn't accurate. Here:

response = requests.get(JSON_GET_URL)
json_data = response.json()

you would need it between. .json() closes the socket (not sure why...)
https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/main/adafruit_requests.py#L324

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@justmobilize that works and returns sockets 1 for each example. very picky about the placement. That's pretty neat!

# SPDX-FileCopyrightText: 2024 DJDevon3
# SPDX-License-Identifier: MIT
# Updated for Circuit Python 9.0
""" WiFi Context Manager Basics Example """

import os

import adafruit_connection_manager
import wifi

import adafruit_requests

# Get WiFi details, ensure these are setup in settings.toml
ssid = os.getenv("CIRCUITPY_WIFI_SSID")
password = os.getenv("CIRCUITPY_WIFI_PASSWORD")

# Initalize Wifi, Socket Pool, Request Session
pool = adafruit_connection_manager.get_radio_socketpool(wifi.radio)
ssl_context = adafruit_connection_manager.get_radio_ssl_context(wifi.radio)
requests = adafruit_requests.Session(pool, ssl_context)
rssi = wifi.radio.ap_info.rssi

JSON_GET_URL = "https://httpbin.org/get"

print(f"\nConnecting to {ssid}...")
print(f"Signal Strength: {rssi}")
try:
    # Connect to the Wi-Fi network
    wifi.radio.connect(ssid, password)
except OSError as e:
    print(f"❌ OSError: {e}")
print("✅ Wifi!\n")

print("-" * 40)
# This method requires an explicit close
print("Explicit Close Example")
response = requests.get(JSON_GET_URL)
used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
print(f" | Current used sockets: {used_sockets}")
json_data = response.json()
if response.status_code == 200:
    print(f" | 🆗 Status Code: {response.status_code}")
else:
    print(f" |  | Status Code: {response.status_code}")
headers = json_data["headers"]
date = response.headers.get("date", "")
print(f" |  | Response Timestamp: {date}")

# Close response manually (prone to mid-disconnect socket errors, out of retries)
response.close()
print(f" | ✂️ Disconnected from {JSON_GET_URL}")

# Shows the response/socket has been closed.
used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
print(f" | Current used sockets: {used_sockets}")

# This example shows json_data still available after response close
content_type = response.headers.get("content-type", "")
print(f" | Content-Type: {content_type}")

print("\nversus\n")

print("-" * 40)
# Closing response is included automatically using "with"
print("Context Manager WITH Example")
response = requests.get(JSON_GET_URL)
# Wrap a request using a with statement
with requests.get(JSON_GET_URL) as response:
    date = response.headers.get("date", "")
    used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
    print(f" | Current used sockets: {used_sockets}")
    json_data = response.json()
    if response.status_code == 200:
        print(f" | 🆗 Status Code: {response.status_code}")
    else:
        print(f" |  | Status Code: {response.status_code}")
    headers = json_data["headers"]
    print(f" |  | Response Timestamp: {date}")

# Notice there is no response.close() here
# It's handled automatically in a with statement
# This is the better way.
print(f" | ✂️ Disconnected from {JSON_GET_URL}")

# Shows the response/socket has been closed.
used_sockets = len([socket for socket, free in requests._connection_manager._available_socket.items() if free is False])
print(f" | Current used sockets: {used_sockets}")

# This example shows json_data still available outside of with.
content_type = response.headers.get("content-type", "")
print(f" | Content-Type: {content_type}")


print("\nBoth examples are functionally identical")
print(
    "However, a with statement is more robust against disconnections mid-request "
    + "and automatically closes the response."
)
print("Using with statements for requests is recommended\n\n")

How about something like this that shows the socket is closing properly?

Connecting to WiFi...
Signal Strength: -56
✅ Wifi!

----------------------------------------
Explicit Close Example
 | Current used sockets: 1
 | 🆗 Status Code: 200
 |  | Response Timestamp: Thu, 11 Apr 2024 17:04:34 GMT
 | ✂️ Disconnected from https://httpbin.org/get
 | Current used sockets: 0
 | Content-Type: application/json

versus

----------------------------------------
Context Manager WITH Example
 | Current used sockets: 1
 | 🆗 Status Code: 200
 |  | Response Timestamp: Thu, 11 Apr 2024 17:04:42 GMT
 | ✂️ Disconnected from https://httpbin.org/get
 | Current used sockets: 0
 | Content-Type: application/json

Both examples are functionally identical
However, a with statement is more robust against disconnections mid-request and automatically closes the response.
Using with statements for requests is recommended

@DJDevon3
Copy link
Contributor Author

@justmobilize Might have to skip adding it to this example. Pylint throws a lot of important sounding errors for this one

************* Module requests_wifi_context_manager_basics
examples\wifi\requests_wifi_context_manager_basics.py:45:28: W0212: Access to a protected member _available_socket of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:45:28: W0212: Access to a protected member _connection_manager of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:68:28: W0212: Access to a protected member _available_socket of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:68:28: W0212: Access to a protected member _connection_manager of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:96:32: W0212: Access to a protected member _available_socket of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:96:32: W0212: Access to a protected member _connection_manager of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:119:28: W0212: Access to a protected member _available_socket of a client class (protected-access)
examples\wifi\requests_wifi_context_manager_basics.py:119:28: W0212: Access to a protected member _connection_manager of a client class (protected-access)

@DJDevon3
Copy link
Contributor Author

ignore previous commit title and description. that did not work out.

@anecdata
Copy link
Member

anecdata commented Apr 11, 2024

Subtle point, and I'm not 100% positive about this, but it may not be ideal to use response after explicit close or implicit context manager with exits. Responses are a special case the way they're written, my understanding is that typically if you want to use some data from the object being closed with a context manager block (or after explicit close), you'd assign it to a variable that lives beyond that block. Or, just complete the work inside the block. It works in Python too, so 🤷, but in general that kind of use may not work.

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@anecdata I completely agree and the fact that it even worked without assigning it to a variable as a buffer is a little disconcerting. Someone might need to look into ensuring the response variable is zero'd out upon close. I'm not sure if that's possible from the library side as response can be named any variable. Maybe it might be a good idea to set response = "" after or some other comparable approach?

I suppose response is the buffer variable so it might actually be working as intended. My reuse of the same variable names between 2 different request gets is a bad example. Will fix but this has brought up a good point of discussion.

@justmobilize
Copy link
Collaborator

This:

with thing() as var:

creates var. it's the same as:

var = thing()

just that if you use with you can do clean-up

It's hard in things like this, because people who aren't python developers could walk away with assumptions on what you can do with with

@justmobilize
Copy link
Collaborator

@DJDevon3 separately, you can add the pylint disable to the top of the example if you want to leave those in

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 11, 2024

@justmobilize Nah I think it's outside the intent for a basic script anyway. It's neat to know you can do that. Yes this isn't a real context manager with enter and exit but it's a start. I was advised to use with in the meeting "in the weeds" so that's where all the examples are headed.

@justmobilize
Copy link
Collaborator

I will say this example isn't valid though. Since response.json() calls response.close() so it's closed automatically

@DJDevon3
Copy link
Contributor Author

@justmobilize yeah it's putting it into json_data so of course it's going to be available after. 🤦 Back to the drawing board.

@justmobilize
Copy link
Collaborator

So it depends the goal. If you show if it's still in use, then you are in a good spot.

You can also use .text() since that doesn't close.

Or do a partial read, close and show you can't read more...

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 12, 2024

Goal is just to show the different ways of making a request and why using a with statement is recommended. Everything else is secondary. It is likely something that I will refer to often while updating all of the requests examples. If I need it as a reference for most examples going forward then might as well make a basic example for it.

response = requests.get(JSON_GET_URL)
print(" | ✅ Connected to JSON")

json_data = response.json()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am understanding the conversation here correctly I think it makes the most sense to switch reponse.json() here to use response.text() instead. Then if you want you can use json.loads to parse it and get the data out.

That way the one that is illustrating the manual call to close() is serving it's intended purpose about illustrating the necessity to close afterward when the context processor isn't in use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter if I do data = response.text or data = response.json both are available after the explicit close because both are being buffered into the data variable. Can you post an example of what you mean because I'm not understanding the difference in use. Yes .text will print out the raw json data format and .json prints out a parsed version. With regards to .close() I'm not seeing any difference in behavior because the data variable acts as a buffer.

@justmobilize
Copy link
Collaborator

@DJDevon3 and @FoamyGuy with the mass updating to with in #188, this one can probably be closed out?

@DJDevon3
Copy link
Contributor Author

DJDevon3 commented Apr 22, 2024

@justmobilize still working on this one. I'm not sure the example I chose to go with is the best one to demonstrate the difference. It's possible I just don't understand the difference and have been doing things the wrong way all the time? Which oddly enough is the entire point of the example.

@anecdata
Copy link
Member

anecdata commented Apr 22, 2024

The context manager for a Requests response differs a bit from one like (for example) files or sockets.

With files, using a context manager to open a file ensures that the file is closed before fully exiting the with block. You can't read or write a file after it's closed explicitly, or implicitly by exiting with.

But in the requests case, the context manager is for the response, but the response isn't actually closed like a file is closed. The underlying __exit()__ only calls to either "free" (for re-use) or close the socket used by the response. So the response is still "open" and available after the with block exits, just as it would be by calling response.close(). You can access properties and functions of a response after the response is "closed" explicitly or implicitly by exiting with.

Sockets are hidden from the user in Requests and are managed either directly by Requests or by ConnectionManager. It may not be intuitive to the user that explicitly or implicitly closing the response does no clean-up other than on the hidden socket.

@FoamyGuy
Copy link
Contributor

@anecdata are there more resources used by it that need to be "cleaned up" after?

I actually didn't realize you could still access and use the response after the with block closed. I thought that was like defining a variable inside of an indented block and then trying to use it after or outside of that block.

Now that I know it works like this, I would say it does seem counter-intuitive to me in that regards, but it is also matching the behavior of CPython so I'm inclined to believe it should stay the same rather than change.

My understanding is after the with block closes the only resource being used by the response is the RAM that it occupies. My assumption would be that it becomes illegible for garbage collection if / when response variable is re-assigned to something else i.e. when the next request happens.

@anecdata
Copy link
Member

anecdata commented Apr 22, 2024

@FoamyGuy I agree it's not intuitive, but matches CPython so it's more of something to explain when needed. I think typically the response would get re-assigned next time in the loop, and the prior object could be gc'd. Maybe some folks have a tight memory use case where they'd want to manually clean it up right away.

[The other related (very) unintuitive thing, if I'm reading the code right, is that with sessions, when the response is closed, the socket is "freed" (not closed), meaning it's marked as available for re-use. But even after closing the response (explicitly or implicitly) and freeing the socket, you can get the response text/content/json dynamically from the freed socket. Only when the socket is closed, or re-used by a subsequent response, does it become unavailable to the current response.

My understanding of the socket re-use is to save the time to re-set-up the TCP connection (especially if TLS) when one of the same freed host-port-proto combos is needed again - so in a way I think of this as a network optimization that results in more complex and less intuitive code and behavior.]

@DJDevon3
Copy link
Contributor Author

This was my understanding of it as well. I tried a couple iterations to not put the response into a buffer and it didn't like that, usually returning response not iterable. Using variable = response.json the variable will hold the response in memory and doing explicit close does not affect it. I thought it would and that's why I started on this example with the understanding it would somehow wipe the buffer variable. @justmobilize If you can come up with an example that demonstrates different behavior using .close() vs with, it would be appreciated. I tried and completely failed at it apparently.

@DJDevon3
Copy link
Contributor Author

I'll close this and will let @justmobilize come up with an example for this in the future... and we will need an example like this in the future.

@DJDevon3 DJDevon3 closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants