Skip to content

feat: introduce batched requests for async-loops #78

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

Merged
merged 2 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,10 @@ In addition, you can set the `filesToDelete` property as an array of strings (fi
"ignoreDeletionFailures": true,
}
```

- If `batchSize` is set, then file deletions and file uploads will use batched concurrent requests as opposed to iterating through them. This can be helpful for uploading many small files. Beware of your Github usage limits.

```javascript
{
"batchSize": 10
}
119 changes: 72 additions & 47 deletions create-or-update-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ module.exports = function(octokit, opts) {
return reject("No changes provided");
}

if (!opts.batchSize) {
opts.batchSize = 1;
}

if (typeof opts.batchSize !== "number") {
return reject(`batchSize must be a number`);
}

// Destructuring for easier access later
let {
owner,
Expand All @@ -22,7 +30,8 @@ module.exports = function(octokit, opts) {
createBranch,
committer,
author,
changes
changes,
batchSize
} = opts;

let branchAlreadyExists = true;
Expand Down Expand Up @@ -81,60 +90,68 @@ module.exports = function(octokit, opts) {
const treeItems = [];
// Handle file deletions
if (hasFilesToDelete) {
for (const fileName of change.filesToDelete) {
const exists = await fileExistsInRepo(
octokit,
owner,
repo,
fileName,
baseTree
for (const batch of chunk(change.filesToDelete, batchSize)) {
await Promise.all(
batch.map(async fileName => {
const exists = await fileExistsInRepo(
octokit,
owner,
repo,
fileName,
baseTree
);

// If it doesn't exist, and we're not ignoring missing files
// reject the promise
if (!exists && !change.ignoreDeletionFailures) {
return reject(
`The file ${fileName} could not be found in the repo`
);
}

// At this point it either exists, or we're ignoring failures
if (exists) {
treeItems.push({
path: fileName,
sha: null, // sha as null implies that the file should be deleted
mode: "100644",
type: "commit"
});
}
})
);
}
}

// If it doesn't exist, and we're not ignoring missing files
// reject the promise
if (!exists && !change.ignoreDeletionFailures) {
return reject(
`The file ${fileName} could not be found in the repo`
for (const batch of chunk(Object.keys(change.files), batchSize)) {
await Promise.all(
batch.map(async fileName => {
const properties = change.files[fileName] || "";

const contents = properties.contents || properties;
const mode = properties.mode || "100644";
const type = properties.type || "blob";

if (!contents) {
return reject(`No file contents provided for ${fileName}`);
}

const fileSha = await createBlob(
octokit,
owner,
repo,
contents,
type
);
}

// At this point it either exists, or we're ignoring failures
if (exists) {
treeItems.push({
path: fileName,
sha: null, // sha as null implies that the file should be deleted
mode: "100644",
type: "commit"
sha: fileSha,
mode: mode,
type: type
});
}
}
}

for (const fileName in change.files) {
const properties = change.files[fileName] || "";

const contents = properties.contents || properties;
const mode = properties.mode || "100644";
const type = properties.type || "blob";

if (!contents) {
return reject(`No file contents provided for ${fileName}`);
}

const fileSha = await createBlob(
octokit,
owner,
repo,
contents,
type
})
);

treeItems.push({
path: fileName,
sha: fileSha,
mode: mode,
type: type
});
}

// no need to issue further requests if there are no updates, creations and deletions
Expand Down Expand Up @@ -278,3 +295,11 @@ async function loadRef(octokit, owner, repo, ref) {
// console.log(e);
}
}

const chunk = (input, size) => {
return input.reduce((arr, item, idx) => {
return idx % size === 0
? [...arr, [item]]
: [...arr.slice(0, -1), [...arr.slice(-1)[0], item]];
}, []);
};
5 changes: 5 additions & 0 deletions create-or-update-files.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ test(`empty parameter (changes)`, () => {
expect(run(body)).rejects.toEqual(`No changes provided`);
});

test(`non-number batchSize (changes)`, () => {
const body = { ...validRequest, batchSize: "5" };
expect(run(body)).rejects.toEqual(`batchSize must be a number`);
});

test(`branch does not exist, createBranch false`, async () => {
mockGetRef(branch, `sha-${branch}`, false);
const body = { ...validRequest, createBranch: false };
Expand Down