Skip to content

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Apr 19, 2022
Merged

chore(javascript): split build task to avoid memory issue #383

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0748895
chore(javascript): split build task to avoid memory issue
eunjae-lee Apr 15, 2022
c123c63
Merge branch 'main' into chore/js-memory
eunjae-lee Apr 15, 2022
d0c4175
Update clients/algoliasearch-client-javascript/rollup.config.js
eunjae-lee Apr 15, 2022
75d0e81
Merge branch 'main' into chore/js-memory
eunjae-lee Apr 15, 2022
f0f40b7
Merge branch 'main' into chore/js-memory
eunjae-lee Apr 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clients/algoliasearch-client-javascript/.github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:

- name: Build clients
shell: bash
run: yarn build
run: yarn build:all
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Member

@shortcuts shortcuts Apr 15, 2022 •
edited
Loading

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

Copy link
Collaborator

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

Copy link
Member

Choose a reason for hiding this comment

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

Eunjae can IIRC

Copy link
Contributor Author

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.

Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍


- name: Publish to NPM
shell: bash
Expand Down
1 change: 1 addition & 0 deletions clients/algoliasearch-client-javascript/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
],
"scripts": {
"build": "CLIENT=${0:-all} yarn rollup -c rollup.config.js",
"build:all": "./scripts/build_all.sh",
"build:utils": "yarn build utils",
"clean": "rm -rf packages/*/dist || true",
"clean:utils": "yarn workspace @experimental-api-clients-automation/client-common clean && yarn workspace @experimental-api-clients-automation/requester-node-http clean && yarn workspace @experimental-api-clients-automation/requester-browser-xhr clean",
Expand Down
12 changes: 8 additions & 4 deletions clients/algoliasearch-client-javascript/rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,18 @@ function getUtilConfigs() {
];
}

function isClientBuilt(client) {
function shouldBuildUtil(utilClient) {
if (process.env.SKIP_UTILS) {
return false;
}

// Checking existence of `dist` folder doesn't really guarantee the built files are up-to-date.
// However, on the CI, it's very likely.
// For the local environment, we simply return `false`, which will trigger unnecessary builds.
// We can come back here and improve this part (checking if `dist` folder exists and if it's up-to-date).
return process.env.CI
? fs.existsSync(path.resolve('packages', client, 'dist'))
: false;
? !fs.existsSync(path.resolve('packages', utilClient, 'dist'))
: true;
}

function getPackageConfigs() {
Expand Down Expand Up @@ -168,7 +172,7 @@ function getPackageConfigs() {
});

return [
...getUtilConfigs().filter((config) => !isClientBuilt(config.package)),
...getUtilConfigs().filter((config) => shouldBuildUtil(config.package)),
...configs,
];
}
Expand Down
11 changes: 11 additions & 0 deletions clients/algoliasearch-client-javascript/scripts/build_all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash
set -e

yarn build utils

PACKAGES_EXCEPT_FOR_UTILS=$(ls ./packages | grep -v -E "(client-common|requester-)")

for CLIENT in $PACKAGES_EXCEPT_FOR_UTILS
do
SKIP_UTILS=true yarn build $CLIENT
done