-
Notifications
You must be signed in to change notification settings - Fork 21
chore(javascript): split build task to avoid memory issue #383
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
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
@@ -27,7 +27,7 @@ jobs: | |||
|
|||
- name: Build clients | |||
shell: bash | |||
run: yarn build | |||
run: yarn build:all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I asked on slack, if this split part works, why don't we use a matrix? This way there's no conditional stuff added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean stuff like createMatrix
? It's because they won't be copied into the JS repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just mean creating a GitHub matrix, so each jobs are run in parallel in their own runners
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And part 2 of what I said on slack, runners are not shared with the JavaScript repository, which might also not help the memory limit as it runs on the default GH runners. (TBC)
compare https://github.com/algolia/api-clients-automation/settings/actions/runners to https://github.com/algolia/algoliasearch-client-javascript/settings/actions/runners
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only admin can see that page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eunjae can IIRC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, now I see what you mean by GitHub matrix. But how does it solve
This way there's no conditional stuff added
this problem?
And since we split the build into many, we don't have memory issue any more. Maximum 2~300MB is enough now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I managed to make the CLI work as before so you can ignore my comment if you prefer going with the bash solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Co-authored-by: Pierre Millot <[email protected]>
* chore(javascript): split build task to avoid memory issue * Update clients/algoliasearch-client-javascript/rollup.config.js Co-authored-by: Pierre Millot <[email protected]> Co-authored-by: Pierre Millot <[email protected]>
🧭 What and Why
Changes included:
The JS build process dies due to heap memory limitation (more precisely because of how rollup adds up memory usage as it iterates over configs).
This PR splits the
yarn build
into multiple. Every time a process ends, it will release memory, and this will fix our issue.The failed GHA: https://github.com/algolia/algoliasearch-client-javascript/runs/6035896998?check_suite_focus=true