feat: Make additional function attributes available via functions manifest #1221

jackiewmacharia · 2022-10-14T12:12:47Z

🎉 Thanks for submitting a pull request! 🎉

Summary

Issue: https://github.com/netlify/pod-compute/issues/192
Issue: https://github.com/netlify/pod-compute/issues/156
Makes displayName, nodeBundler and isInternalFunction available to the API

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 ✅

A picture of a cute animal (not mandatory, but encouraged)

github-actions · 2022-10-14T12:14:44Z

⏱ Benchmark results

Comparing with de48712

largeDepsEsbuild: 3s

⬆️ 32.68% increase vs. de48712

^                                                    3s                      3s   
│                                                   ┌──┐                    ┌──┐  
│                                                   |  |    2.8s            |▒▒|  
│           2.6s                                    |  |    ┌──┐            |▒▒|  
│           ┌──┐                                    |  |    |  |            |▒▒|  
│ ──────────┼──┼────2.2s────────────────────────────┼──┼────┼──┼────────────|▒▒|──
│           |  |    ┌──┐    2.1s    2.1s    2.1s    |  |    |  |            |▒▒|  
│    2s     |  |    |  |    ┌──┐    ┌──┐    ┌──┐    |  |    |  |     2s     |▒▒|  
│   ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-9 (9160543): 2s
T-8 (298c68c): 2.6s
T-7 (34309e2): 2.2s
T-6 (2449c71): 2.1s
T-5 (40e5a58): 2.1s
T-4 (d4327a3): 2.1s
T-3 (a894743): 3s
T-2 (977a39d): 2.8s
T-1 (de48712): 2s
T (current commit): 3s

largeDepsNft: 11.9s

⬆️ 36.19% increase vs. de48712

^          13.4s                                                                  
│           ┌──┐                                                                  
│           |  |                                   11.8s                   11.9s  
│           |  |                                    ┌──┐                    ┌──┐  
│           |  |   10.6s                            |  |   10.9s            |▒▒|  
│           |  |    ┌──┐   10.3s                    |  |    ┌──┐            |▒▒|  
│ ──9.4s────┼──┼────┼──┼────┌──┐────9.7s────────────┼──┼────┼──┼────────────|▒▒|──
│   ┌──┐    |  |    |  |    |  |    ┌──┐            |  |    |  |            |▒▒|  
│   |  |    |  |    |  |    |  |    |  |     8s     |  |    |  |            |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    ┌──┐    |  |    |  |    7.6s    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    ┌──┐    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-9 (9160543): 9.4s
T-8 (298c68c): 13.4s
T-7 (34309e2): 10.6s
T-6 (2449c71): 10.3s
T-5 (40e5a58): 9.7s
T-4 (d4327a3): 8s
T-3 (a894743): 11.8s
T-2 (977a39d): 10.9s
T-1 (de48712): 7.6s
T (current commit): 11.9s

largeDepsZisi: 21.6s

⬆️ 30.92% increase vs. de48712

^                                                  21.2s                   21.6s  
│                                                   ┌──┐   20.7s            ┌──┐  
│                                                   |  |    ┌──┐            |▒▒|  
│          18.9s                                    |  |    |  |            |▒▒|  
│           ┌──┐   16.8s                            |  |    |  |            |▒▒|  
│ ──────────┼──┼────┌──┐───────────────────15.8s────┼──┼────┼──┼────────────|▒▒|──
│           |  |    |  |   15.6s   15.3s    ┌──┐    |  |    |  |   14.9s    |▒▒|  
│  14.4s    |  |    |  |    ┌──┐    ┌──┐    |  |    |  |    |  |    ┌──┐    |▒▒|  
│   ┌──┐    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴────┴──┴──>
    T-9     T-8     T-7     T-6     T-5     T-4     T-3     T-2     T-1      T

Legend

T-9 (9160543): 14.4s
T-8 (298c68c): 18.9s
T-7 (34309e2): 16.8s
T-6 (2449c71): 15.6s
T-5 (40e5a58): 15.3s
T-4 (d4327a3): 15.8s
T-3 (a894743): 21.2s
T-2 (977a39d): 20.7s
T-1 (de48712): 14.9s
T (current commit): 21.6s

src/runtimes/constants.ts

src/config.ts

src/runtimes/node/bundlers/esbuild/bundler_target.ts

src/utils/check_is_internal_function.ts

tests/main.test.ts

… functions folder

…playName property

…isplayName property

…ions

… and resolve

README.md

tests/zip_function.test.ts

…nalFunction

eduardoboucas

I left some comments that I think we should address (the path normalisation changes), but other than that it's looking great! ✨

README.md

src/runtimes/node/bundlers/zisi/resolve.ts

src/runtimes/node/utils/module.ts

tests/main.test.ts

…ternalSrcFolder

…fails" This reverts commit b444d1c.

eduardoboucas

🚀

changed a lot!

jackiewmacharia changed the title ~~Make additional function attributes available via functions manifest~~ (feat): Make additional function attributes available via functions manifest Oct 14, 2022

jackiewmacharia changed the title ~~(feat): Make additional function attributes available via functions manifest~~ feat: Make additional function attributes available via functions manifest Oct 14, 2022

github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 14, 2022

khendrikse self-assigned this Jan 9, 2023

khendrikse force-pushed the feat/pass-additional-function-attributes branch from c656390 to f5993ec Compare January 9, 2023 15:22

khendrikse marked this pull request as ready for review January 10, 2023 13:47

khendrikse requested a review from a team January 10, 2023 13:47

khendrikse marked this pull request as draft January 10, 2023 13:47

khendrikse marked this pull request as ready for review January 10, 2023 13:49

eduardoboucas reviewed Jan 10, 2023

View reviewed changes

src/runtimes/constants.ts Outdated Show resolved Hide resolved

danez previously requested changes Jan 10, 2023

View reviewed changes

src/config.ts Outdated Show resolved Hide resolved

src/runtimes/node/bundlers/esbuild/bundler_target.ts Outdated Show resolved Hide resolved

src/utils/check_is_internal_function.ts Outdated Show resolved Hide resolved

tests/main.test.ts Outdated Show resolved Hide resolved

khendrikse force-pushed the feat/pass-additional-function-attributes branch 3 times, most recently from 731567f to dd92b53 Compare January 11, 2023 10:10

khendrikse marked this pull request as draft January 11, 2023 12:28

jackiewmacharia and others added 14 commits January 11, 2023 15:37

feat: pass additional-function-attributes

209b447

chore: make attributes available to functions manifest

61a564e

fix: fix linting

b39c535

fix: use optional chaining

5f6e420

fix: add ext

7a55dc3

fix: fix tests

8a0a033

fix: change internal directory name in constants.ts

06e2c09

feat: add test for go functions with displayName and from an internal…

ee0c043

… functions folder

feat: add test for rust functions build from internal folder with dis…

ccadb79

…playName property

feat: add test for nodejs functions build from internal folder with d…

be4d323

…isplayName property

test: figure out windows breakn

4901e39

fix: take windows paths into account when checking for internal funct…

4380c3f

…ions

test: add unit test for checkIsInternalFunction

67c39ea

docs: add new return values

825cf20

khendrikse added 4 commits January 11, 2023 15:37

test: move tests into a better fixtures structure

c9c31bf

fix: use unixify for checkIsInternalFunction, config, bundler, module…

0b617d0

… and resolve

feat: add internalFunctionsFolder option to zipfunctionoptions

e7b17b5

test: fix tests

9f5b99b

khendrikse force-pushed the feat/pass-additional-function-attributes branch from 763f07d to 9f5b99b Compare January 11, 2023 14:38

test: add test for zipFunction and code to support that as well

8858617

khendrikse marked this pull request as ready for review January 11, 2023 15:27

khendrikse requested review from eduardoboucas and danez January 11, 2023 15:27

eduardoboucas reviewed Jan 11, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

tests/zip_function.test.ts Outdated Show resolved Hide resolved

test: change toBeTruthy to toBe(true) and isInternalFunction to inter…

196933d

…nalFunction

khendrikse requested a review from eduardoboucas January 11, 2023 16:14

khendrikse added 2 commits January 11, 2023 17:21

chore: change displayName input to name

ab7f943

chore: change internalFunction to isInternal

8e02225

eduardoboucas reviewed Jan 11, 2023

View reviewed changes

khendrikse added 3 commits January 12, 2023 11:01

chore: revert unixify, rename tests and internalFunctionsFolder to in…

17150e2

…ternalSrcFolder

test: revert unixify for module to see if windows test still fails

b444d1c

Revert "test: revert unixify for module to see if windows test still …

4e77889

…fails" This reverts commit b444d1c.

khendrikse requested a review from eduardoboucas January 12, 2023 10:44

eduardoboucas approved these changes Jan 12, 2023

View reviewed changes

Merge branch 'main' into feat/pass-additional-function-attributes

be0654d

khendrikse merged commit ea4008e into main Jan 12, 2023

khendrikse deleted the feat/pass-additional-function-attributes branch January 12, 2023 11:05

token-generator-app bot mentioned this pull request Jan 12, 2023

chore(main): release 8.4.0 #1309

Merged

khendrikse mentioned this pull request Jan 12, 2023

feat: Pass additional functions attributes to API from manifest netlify/open-api#397

Closed

This was referenced Feb 8, 2023

feat: add generator as an option to config and save it in the manifest and remove isInternal #1341

Merged

fix: add test for generator field and remove internalSrcFolder usage netlify/build#4857

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Make additional function attributes available via functions manifest #1221

feat: Make additional function attributes available via functions manifest #1221

jackiewmacharia commented Oct 14, 2022 •

edited by khendrikse

github-actions bot commented Oct 14, 2022 •

edited

eduardoboucas left a comment

eduardoboucas left a comment

feat: Make additional function attributes available via functions manifest #1221

feat: Make additional function attributes available via functions manifest #1221

Conversation

jackiewmacharia commented Oct 14, 2022 • edited by khendrikse

Summary

github-actions bot commented Oct 14, 2022 • edited

⏱ Benchmark results

largeDepsEsbuild: 3s

largeDepsNft: 11.9s

largeDepsZisi: 21.6s

eduardoboucas left a comment

Choose a reason for hiding this comment

eduardoboucas left a comment

Choose a reason for hiding this comment

jackiewmacharia commented Oct 14, 2022 •

edited by khendrikse

github-actions bot commented Oct 14, 2022 •

edited