-
Notifications
You must be signed in to change notification settings - Fork 99
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
Added ability to parse json #73
Conversation
This pull request should add support for parsing json so partially resolves one of the issues in #61 |
Thanks for the submission.
But did you read that ticket? Because if you did (and also read README), you'd know that Picoweb is minimalist web framework which doesn't duplicate functionality already available in other modules/libraries. You can parse json using |
Then by your own definition the function should not parse the form data since the user could just call parse_qs(data). The problem is that while parsing json is supported by another module the pico web module has functions to help with reading the response however the form data is read into a dictionary and parsed as a form instead of providing the raw string. I would propose that if the framework is not going to provide a json read function then the function below should be modified so that the raw response is made available to be used def read_form_data(self): Here are a few proposals Option 1 - Save the (Raw Data) as a property on the Request so that it can be parsed by an external library without having to write a helper function for pico web module. def read_form_data(self): Option 2 - Dont parse the data but return the raw response and let the calling application determine if its form data or json data. def read_form_data(self): Using option 1 the web application can parse json by req.read_form_data() Using Option 2 The web application can parse the json by calling req.read_form_data() I would lean towards Option 1 because it maintains backwards compatibility without adding any lines of code but makes it very easy to parse the json while not having to re-write the code that reads the string from the request. |
One other point is that if its such a bad idea to have a function in the HTTPRequest to parse the JSON Body which in todays day is a requirement on almost any web application then why is there a function called Jsonify provided by pico web which takes the dictionary and writes it to the response as Json? Couldn’t the web application just call response.awrite(json.dumps(dict))? |
Another 3rd option would be to define the function that reads the form data in such a way that it takes a parse data function as an agreement. def read_form_data(self, parseFunction): This allows the calling application to define how the data should be parsed (Json, Bson, Form Data, ect....) While still not having to deal with reading the raw response from the request. |
My own definition is very simple: Be conservative with both adding new things, and removing existing things. And as this is minimalist project, be extra conservative with adding new things.
Dealing with HTML form data is what almost any standard (classic) web app would do, and hence convenience function is provided.
Because it's pretty common usecase to develop a webapp which exports REST API with JSON responses. So, a convenience function for that was provided.
Please understand that there's no need to modify anything. Instead you do:
If you find that almost all your webapps use such an extended subclass, you can put it in a separate module for easy reuse, and even share it with others and maintain it yourself (answer questions, etc.). I encourage all that. All that was mentioned in #61 (I keep it open exactly to let people know all that without repeating it again and again). But picoweb itself is intended to stay minimalist, and allow to develop small/simple webapps without adding much extra code overhead, while also being extensible (like above) to allow developing more complex webapps. Thanks for the discussion. |
No description provided.