[GSOC 2023] Add Configurable Content Type Parsing and Modernise Request Object

Django Internals Mentorship
1

Hello Everyone,

Thank you for selecting my proposal and providing me with this opportunity to work with you @nessita @smithdc1 @adamchainz !

I am starting this thread to monitor my progress as per the suggestion of Natalia. I will also try to write blogs detailing the work and progress I have made every 2 weeks.

Work Done so Far:
As per discussions with Carlton in this thread, I was able to complete the pre-GSoC target of this project, which was adding a JSON parser. The PR was made in Carlton’s fork of django. Link to PR

Current:
Moving forward I would like to open a WIP PR in the main Django repo, working on the latest branch. Part 1 of my project is already done, so I will get to work on the second part (adding the custom parser API).

Due to unforeseen circumstances, I wasn’t able to get a proper review of my proposal and the proposed implementation during the application period. It would be helpful if the mentors can give their thoughts and if any changes are required. Gist of my proposal

I’m excited to continue working on this project and contribute to Django’s development :rocket:.

Warm Regards,
Abhinav

4 Likes
2

Hi @SirAbhi13 :wave:

Congratulations again on having your proposal accepted this year. :tada:

Thank you for your links to previous work, they are very useful. I’ll have a read in the coming days. In the meantime, I agree it would be good to move that PR from Carlton’s fork to the main Django repo to improve visibility. (This PR is the first step in your project?)

I think at this stage you are several steps ahead of us. It would therefore be useful if you could help me catch-up to where you are with your thinking. I think if you could articulate your plan on what you see are the key steps that would be helpful, this could be a summary of your milestones in your proposal, assuming that’s still your plan.

I’d also be interested in understanding if there are the any dependencies between changes (e.g. do we need custom parsing before renaming attribute names?), what design decisions need to be made, what support / reviews you’re expecting and so on. The aim here is for you to help manage us to some extent, tell us when you’re aiming for / need reviews for to keep the project on plan.

Welcome aboard! :ship:

1 Like
3

Welcome @SirAbhi13 :slight_smile: I’m looking forward to seeing how you progress with the work.

Some notes on your proposal.

** 3.1.1 Rename HttpRequest.POST to HttpRequest.data and route HttpRequest.POST API calls to HttpRequest.data.**

I don’t think we can route POST to data - it needs to be left as-is. It could lead to security problems in many cases if code that expects only form data starts also accepting JSON, or whatever other parsers are configured, as well.

The idea of request.data is that it’s a new content-type aware attribute.

3.1.4 Remodel Form Parsing logic

Move the form parsing logic from the HttpRequest class to the parsers.py module and also model it after the new API design.

I am not sure this is entirely feasible. The new parsers API should be for setting request.data only. But multipart form parsing necessitates parsing both form data (request.POST) and files (request.FILES). We should not combine these into a single dictionary.

Previously, Django had request.REQUEST which merged request.GET and request.POST, leading to confusion and security problems. This was deprecated in 1.7: Django 1.7 release notes | Django documentation | Django . We don’t want to repeat that kind of mistake with the new design.

Also from your proposal:

If time permits, I would like to contribute to django-upgrade project to see if I can write code to help people migrate their code to the new design.

Absolutely. It will be a good stretch goal if you can get to that point.

2 Likes
4

Hi @SirAbhi13 hope you’ve had a good week.

Just wondering if you’ve any thoughts on the above?

Do let us know if you need any support.

P.s. looking forward to reading your first blog post!

1 Like
5

Hi Adam, sorry for the late response.
Here are some of my thoughts on the points you raised.

request.POST will still be a separate property that will exist in the request object. Whenever reqest.POST is called, the content-type header of that request object will be checked, if it is a request containing form data, the request.data QueryDict will be returned wherever the request.POST API call is made. If the content-type is something else, an empty QueryDict will be returned. I think this check will prevent any wrong data going at places where only form data is expected. Please do correct me if this line of thinking is wrong. As far as I can see, this will mimic the current behavior of the _load_post_and_files() method in request.py

I was of the opinion since we will create a parsers.py file anyway, it would be better IMO to move all the request parsing logic in one place. This will include the file parsing logic in the multipart form parsing too. I didn’t intend to combine both into a single dictionary. This was just a suggestion and I am open to suggestions on this issue for the best course of action. I understand it will be easier to leave the existing design as it is. I will have to think if we don’t change it, how will the multipart parsing will be handled along with the new design.

Thank you for bringing this to my atttention! I will surely keep this in mind and move forward without bringing regressions in the new design.

6

Hi David, I hope you have had a great week too!

I will give a detailed response to your questions on this weekend. I have had my mid-sem exams this week and wasn’t able to do much. I will start working on my project from this Saturday and will keep everyone in the loop of my next steps.

Will write my blog by this weekend and will share it here!

7

Hi @SirAbhi13! How are things going?

We were wondering if you had any chance to write a blog post or similar with details of your work/investigation so far. Are you block in any way? How can we help?

2 Likes
8

Hi all,
I have written an introductory blog post about getting selected in to GSoC’23. Please have a look and let me know your thoughts! I will write more blogs after each month to recap what work has been done so far.

The way I have planned out this project is to add the implementation of the custom parser and write all the supporting methods needed for it, including the JSONParser(), followed by the tests of these additions.
The key steps are:
Add the logic for the Configurable Parse API, this will include makina parsers.py file, and adding support methods for this logic to work in request.py file (examples are initialisation of parser classes, checking of content-type, etc)

The JSONParser will be added with the above implementation itself.

After at least the skeleton of this is implemented, i will start writing tests.

This will be the task at hand till the first evaluation.

After first evaluation, the goal will be to polish the changes and do a sanity check of the implementation (which will be done before too, but a final check before moving on with the final part of the project).

These changes are not dependent on each other and can be done in any order, but IMO, it would be better to implement the custom parsing part before renaming attribute names.

Regarding the design, I am confident in the methodology I have proposed in my proposal. The support I need which I can think of is perhaps design choices which can be myopic and your large experience and familiarity with Django ecosystem and of the codebase itself can help me come up with a better solution. I might also need little help getting 100% test coverage of the implementation of the custom parser.

Hi @nessita, everything is on track!

I have been pondering and reading up on tests and small changes that will be needed to complete this project.

I will open a draft PR shortly with initial changes and will commit the changes for Custom Parser bit by bit.

1 Like
9

Hi all,

I managed to have a brief chat this week with Carlton and Adam about this at DjangoCon Europe. My two key take aways were.

  1. We must not change any of the existing behaviour. Must make sure we have good test coverage. New behaviour should be accessed via the new API.

  2. Multipart parsing. Should be able to submit say text, json and a file as separate parts and they must all get parsed to their correct type.

1 Like
10

It was good to chat to you all earlier.

One additional thought I’ve had this evening is about performance. This code will get used a lot therefore we need to be mindful of any impact to performance. There’s the django-asv repo where we could review/add benchmarks to test for any regressions (or improvements).

There’s some benchmarks already in the suite. I’m not sure if this is enough.