Skip to content
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

fix: correctly support multiValueHeaders in go functions during development #4618

Merged
merged 9 commits into from
May 25, 2022

Conversation

danez
Copy link
Contributor

@danez danez commented May 19, 2022

Summary

Fixes #4575

multiValueHeaders was simply not forwarded from the response of the go function

Side note: rust functions have the same problem, but I did not want to fix this in one PR. Should I apply this fix also to rust?


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

cat_hat_and_sunglasses_large_2c5d1dfe-fe71-4cb9-a5fc-d5e7a658e10c_large

…elopment

`multiValueHeaders` was simply not forwarded from the response of the go function
@danez danez added type: bug code to address defects in shipped code area: functions labels May 19, 2022
@danez danez requested a review from a team May 19, 2022 17:25
@github-actions
Copy link

github-actions bot commented May 19, 2022

📊 Benchmark results

Comparing with 441d7ff

Package size: 287 MB

(no change)

^  286 MB  286 MB  285 MB  285 MB  285 MB  285 MB  285 MB  285 MB  285 MB  285 MB  285 MB          287 MB 
│   ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐    ┌──┐   274 MB   ┌──┐  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-12    T-11    T-10    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T    
Legend

@danez
Copy link
Contributor Author

danez commented May 19, 2022

🤔 The automation doesn't seem to be sure if I am a contributor 😁

eduardoboucas
eduardoboucas previously approved these changes May 20, 2022
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

Also, to your question:

Side note: rust functions have the same problem, but I did not want to fix this in one PR. Should I apply this fix also to rust?

Yes, please! 🙌🏻

tests/unit/lib/functions/runtimes/go/index.test.js Outdated Show resolved Hide resolved
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
@danez danez requested a review from eduardoboucas May 20, 2022 14:46
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@danez danez enabled auto-merge (squash) May 20, 2022 19:54
@danez danez disabled auto-merge May 25, 2022 17:28
@danez danez added the automerge Add to Kodiak auto merge queue label May 25, 2022
@kodiakhq kodiakhq bot merged commit f8e692f into main May 25, 2022
@kodiakhq kodiakhq bot deleted the dt/4575/multiValueHeadersSupportInGo branch May 25, 2022 18:44
This was referenced May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: functions automerge Add to Kodiak auto merge queue type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Golang functions: Multi value headers not working in local development
2 participants