search

sparckles / Robyn

Robyn is a Super Fast Async Python Web Framework with a Rust runtime.
https://robyn.tech/
BSD 2-Clause "Simplified" License
4.29k stars 221 forks source link

Add default None to QueryParams get fn #780

Closed StandinKP closed 5 months ago

StandinKP commented 6 months ago

Description

This PR fixes #763

vercel[bot] commented 6 months ago

@StandinKP is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

StandinKP commented 6 months ago

I am now matching the default as well to return either None or the default value. So if you don't pass the default param it will return the None. I can try to add a test case if we have some for testing the query params. I can see test_get_request and other similar tests. Do you want me to add it there?

sansyrox commented 6 months ago

I am now matching the default as well to return either None or the default value. So if you don't pass the default param it will return the None. I can try to add a test case if we have some for testing the query params. I can see test_get_request and other similar tests. Do you want me to add it there?

Yes, please do try it with test cases. As both of the diffs are doing the same things. Do give it a thought, if it still doesn't make a lot of sense then I will be happy to explain :D

StandinKP commented 6 months ago

Yes, please do try it with test cases. As both of the diffs are doing the same things. Do give it a thought, if it still doesn't make a lot of sense then I will be happy to explain :D

I just checked and you are right both of them do the same output. Both of them return None if we don't pass in the default value. So I have a question what exactly is wrong with the current setup?

StandinKP commented 5 months ago

@sansyrox do you have some suggestion on what needs to be done here? Or should I close this?

sansyrox commented 5 months ago

Hey @StandinKP 👋

Actually, there was a very simple fix to this problem.

sansyrox commented 5 months ago

See this - https://github.com/sparckles/Robyn/pull/787

sansyrox commented 5 months ago

Closing this, as it has been fixed.