feat: Add support for cookie parameters #326
Conversation
dongfangtianyu
force-pushed
the
dev
branch
2 times, most recently
from
95ae020 to
9ce9760
Compare
Codecov Report
@@ Coverage Diff @@
## main #326 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 47 47
Lines 1394 1398 +4
=========================================
+ Hits 1394 1398 +4
Continue to review full report at Codecov.
|
emann
left a comment
emann
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround on this @dongfangtianyu, looking good so far! If you could add some cookie parameters to an endpoint and/or make a new endpoint that uses cookie parameters in end_to_end_tests/openapi.json and run task re that'd be great - it gives us a chance to see the code actually get generated and check that everything is kosher.
|
Thinks , @emann ! |
There was a problem hiding this comment.
Looks good, just one question / suggestion around transforming cookie names. I believe that we don't want to do this the same way we were for headers. I don't really use cookies though so I could be wrong.
Actually, @sweetb probably knows best here.
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} | ||
| {% else %} | ||
| if {{ parameter.python_name }} is not UNSET: | ||
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} |
There was a problem hiding this comment.
I think that cookies are both case-sensitive and don't follow the same kebab-case convention as headers. So we probably want this to be:
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} | |
| {% else %} | |
| if {{ parameter.python_name }} is not UNSET: | |
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} | |
| cookies["{{ parameter.name}}"] = {{ parameter.python_name }} | |
| {% else %} | |
| if {{ parameter.python_name }} is not UNSET: | |
| cookies["{{ parameter.name}}"] = {{ parameter.python_name }} |
There was a problem hiding this comment.
Yes, that's right.
I will update the test case and commit it again
| cookies["{{ parameter.name}}"] = {{ parameter.python_name }} | ||
| {% else %} | ||
| if {{ parameter.python_name }} is not UNSET: | ||
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} |
There was a problem hiding this comment.
| cookies["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} | |
| cookies["{{ parameter.name }}"] = {{ parameter.python_name }} |
Don't you still want parameter.name set here?
There was a problem hiding this comment.
Sorry, I missed that
emann
left a comment
emann
left a comment
There was a problem hiding this comment.
LGTM as well - thanks again @dongfangtianyu!
emann
left a comment
emann
left a comment
There was a problem hiding this comment.
Almost forgot - please add an entry to the changelog and give yourself credit :)
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
Great, thanks everyone! |
4 participants
close #325