Add support for local OPTIONS CORS headers #444
Conversation
Codecov Report
@@ Coverage Diff @@
## master #444 +/- ##
==========================================
+ Coverage 93.61% 93.87% +0.26%
==========================================
Files 18 18
Lines 2864 3153 +289
Branches 375 440 +65
==========================================
+ Hits 2681 2960 +279
- Misses 132 142 +10
Partials 51 51
Continue to review full report at Codecov.
|
|
Great commenting. I wasn't sure if you folks would like the change hence my quick-n-dirty fix. Thanks for reviewing! |
chalice/local.py
Outdated
| cors_headers = cors_config.get_access_control_headers() | ||
|
|
||
| # The keys will be ordered in python 3 but in python 2 they will not | ||
| # be. To assert this header is sent correctly we need to introduce |
There was a problem hiding this comment.
It is not necessarily true about the need to sort this for the assertion. You could always just split the header value by commas and assert that the sorted list is the same, but that is a bit more tedious. I only bring it up because it is a little weird to have this sorted in the core logic when it is only being sorted for the test. I'm about +0 in updating this especially since we do the same logic in the non chalice local tests (i.e. assert the header values in the a split list).
There was a problem hiding this comment.
Talked offline about this. The reason it is being done is that there is not an easy way to parse the raw body and pull out the specific header/value without using something like the email library. Given that this is being ran in chalice local it is fine that it is getting sorted as chalice local is just a test mode. If this was being done in the actual lambda function, it would be something we would not want to do and should add the extra logic in the tests.
Added the Access-Control-Allow-Methods header to the local OPTIONS response. Also updated the tests and added some comments to explain what is going on in the _send_autogen_options_response method since it was a little hard to follow.
Looks like I couldn't push directly to the #437 for some reason so I opened a separate PR.
closes #437
fixes #436
cc @kyleknap @jamesls