-
Notifications
You must be signed in to change notification settings - Fork 32
Add buffer size property #11
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
Conversation
polling code. Minor fixes to documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple suggestions, but maybe @dhalbert may know more about the effects of changing the request buffer size specifically. Any reason this would be a problem?
adafruit_httpserver.py
Outdated
@@ -361,3 +361,25 @@ def poll(self): | |||
# connection reset by peer, try again later. | |||
return | |||
raise | |||
|
|||
@property | |||
def requestbuffersize(self) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snake case will make this a little more readable:
def requestbuffersize(self) -> int: | |
def request_buffer_size(self) -> int: |
Same thing for the setter of course!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the snake case and it's definitely easier to read. I wasn't sure if there were naming standards, so I just mirrored the other function names. I come from a camel case background, but that doesn't seem to apply in python except in class names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! You've got it spot on. I don't think I've ever seen it explicitly said about the project, but generally we follow the PEP8 style guide (unless what we're making a library that should mirror CPython API like howadafruit_logging
mirrors CPython's logging
modules but that doesn't follow PEP8 itself).
examples/httpserver_simplepolling.py
Outdated
@@ -30,6 +30,7 @@ def base(request): # pylint: disable=unused-argument | |||
|
|||
while True: | |||
try: | |||
# do something useful in this section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It make be helpful to give an example of what you mean by "something useful", like "operate hardware" or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be easy to add some more details to that comment.
Add more details to example comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 5.2.5 from 5.2.4: > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#118 from tcfranks/main > Added Black formatting badge Updating https://github.com/adafruit/Adafruit_CircuitPython_Seesaw to 1.11.3 from 1.11.2: > Switched to pyproject.toml > Added Black formatting badge Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 0.4.0 from 0.3.0: > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#11 from karlfl/AddBufferSizeProperty > Changed .env to .venv in README.rst
Add a property with getter/setter to the HTTPServer class to allow adjustment of the incoming buffer size. Developers can use this to read or change the size of the buffer used to store the incoming request data. This is especially useful when the incoming request is larger than the default buffer size. Without this change any incoming data over the default buffer size (1024 bytes) would be truncated.
Also made some adjustments to the documentation. Adding sections for the temperature and polling examples, fixing an auto generated link and add helpful comment to the polling example.
This PR will close #5