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.

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

Allow setting maxFileAdd via env var #295

Merged
merged 11 commits into from
Jan 3, 2023
Merged

Allow setting maxFileAdd via env var #295

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3032066
fix: testing lowering maxFileAdd
iowillhoit Dec 26, 2022
7d3f898
chore(release): 2.2.16-dev.1 [skip ci]
svc-cli-bot Dec 26, 2022
060b821
fix: maxFileAdd env var
iowillhoit Dec 28, 2022
766bb69
Merge branch 'prerelease/max-file-add-test' of github.com:forcedotcom…
iowillhoit Dec 28, 2022
792f5e3
chore(release): 2.2.16-dev.2 [skip ci]
svc-cli-bot Dec 28, 2022
8e3d011
fix: better error message
iowillhoit Dec 28, 2022
8d2f54a
chore(release): 2.2.16-dev.3 [skip ci]
svc-cli-bot Dec 28, 2022
093490f
Update src/shared/localShadowRepo.ts
iowillhoit Jan 3, 2023
b1dc011
chore: version
iowillhoit Jan 3, 2023
b7f8802
chore: dep bumps
iowillhoit Jan 3, 2023
18f526d
refactor: remove nested ternary
iowillhoit Jan 3, 2023
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
9 changes: 7 additions & 2 deletions .github/workflows/onPushToMain.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ name: version, tag and github release

on:
push:
branches: [main]

branches:
- main
- prerelease/*
tags-ignore:
- '*'
jobs:
release:
uses: salesforcecli/github-workflows/.github/workflows/githubRelease.yml@main
secrets: inherit
with:
prerelease: ${{ github.ref_name != 'main' }}

# most repos won't use this
# depends on previous job to avoid git collisions, not for any functionality reason
Expand Down
15 changes: 13 additions & 2 deletions .github/workflows/onRelease.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: publish

on:
release:
types: [released]
types: [published]
# support manual release in case something goes wrong and needs to be repeated or tested
workflow_dispatch:
inputs:
Expand All @@ -11,9 +11,20 @@ on:
type: string
required: true
jobs:
getDistTag:
outputs:
tag: ${{ steps.distTag.outputs.tag }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.event.release.tag_name || inputs.tag }}
- uses: salesforcecli/github-workflows/.github/actions/getPreReleaseTag@main
id: distTag
npm:
uses: salesforcecli/github-workflows/.github/workflows/npmPublish.yml@main
needs: [getDistTag]
with:
tag: latest
tag: ${{ needs.getDistTag.outputs.tag || 'latest' }}
githubTag: ${{ github.event.release.tag_name || inputs.tag }}
secrets: inherit
27 changes: 27 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
## [2.2.16-dev.3](https://github.com/forcedotcom/source-tracking/compare/2.2.16-dev.2...2.2.16-dev.3) (2022-12-28)


### Bug Fixes

* better error message ([8e3d011](https://github.com/forcedotcom/source-tracking/commit/8e3d0110c1994fd9389c6896d1a1eef415d87462))



## [2.2.16-dev.2](https://github.com/forcedotcom/source-tracking/compare/2.2.16-dev.1...2.2.16-dev.2) (2022-12-28)


### Bug Fixes

* maxFileAdd env var ([060b821](https://github.com/forcedotcom/source-tracking/commit/060b821587ebc10a41919426617e70045ba6db65))



## [2.2.16-dev.1](https://github.com/forcedotcom/source-tracking/compare/2.2.15...2.2.16-dev.1) (2022-12-26)


### Bug Fixes

* testing lowering maxFileAdd ([3032066](https://github.com/forcedotcom/source-tracking/commit/303206643bfa3ef5b6c48f18d4250f3462130da3))



## [2.2.15](https://github.com/forcedotcom/source-tracking/compare/2.2.14...2.2.15) (2022-12-14)


Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@salesforce/source-tracking",
"description": "API for tracking local and remote Salesforce metadata changes",
"version": "2.2.15",
"version": "2.2.16-dev.3",
"author": "Salesforce",
"license": "BSD-3-Clause",
"main": "lib/index.js",
Expand Down
17 changes: 14 additions & 3 deletions src/shared/localShadowRepo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import * as path from 'path';
import * as os from 'os';
import * as fs from 'graceful-fs';
import { NamedPackageDir, Logger, SfError } from '@salesforce/core';
import { env } from '@salesforce/kit';
import * as git from 'isomorphic-git';
import { chunkArray, isLwcLocalOnlyTest, pathIsInFolder } from './functions';

/** returns the full path to where we store the shadow repo */
const getGitDir = (orgId: string, projectPath: string, useSfdxTrackingFiles = false): string => path.join(projectPath, useSfdxTrackingFiles ? '.sfdx' : '.sf', 'orgs', orgId, 'localSourceTracking');
const getGitDir = (orgId: string, projectPath: string, useSfdxTrackingFiles = false): string =>
path.join(projectPath, useSfdxTrackingFiles ? '.sfdx' : '.sf', 'orgs', orgId, 'localSourceTracking');

// filenames were normalized when read from isogit
const toFilenames = (rows: StatusRow[]): string[] => rows.map((row) => row[FILE]);
Expand Down Expand Up @@ -59,7 +61,9 @@ export class ShadowRepo {
this.projectPath = options.projectPath;
this.packageDirs = options.packageDirs;
this.isWindows = os.type() === 'Windows_NT';
this.maxFileAdd = this.isWindows ? 8000 : 15000;

const batchSize = env.getNumber('SFDX_SOURCE_TRACKING_BATCH_SIZE');
this.maxFileAdd = batchSize ? batchSize : this.isWindows ? 8000 : 15000;
Copy link
Contributor

@mshanemc mshanemc Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

rather than nested ternary, you could do something like

Suggested change
this.maxFileAdd = batchSize ? batchSize : this.isWindows ? 8000 : 15000;
this.maxFileAdd = batchSize ?? this.isWindows ? 8000 : 15000;

or just

this.maxFileAdd = env.getNumber('SFDX_SOURCE_TRACKING_BATCH_SIZE') ?? this.isWindows ? 8000 : 15000

or use the "default" 2nd prop of env,

this.maxFileAdd = env.getNumber('SFDX_SOURCE_TRACKING_BATCH_SIZE',  this.isWindows ? 8000 : 15000)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I generally don't like nested ternaries but I like the combination of a nullish coalescing and a ternary less. Mainly because it is not immediately obvious what is being compared. The code you suggested would not work correctly, it would compare the nullish first and use that result for the ternary. If you want to combine the two operators, you'd need to wrap your ternary in parentheses.

// resolves 'tern-true' 👎
console.log('foo' ?? true ? 'tern-true' : 'tern-false');

// resolves 'foo'
console.log('foo' ?? (true ? 'tern-true' : 'tern-false'));

// resolves ""
console.log('' ?? (true ? 'tern-true' : 'tern-false'));

// resolves "tern-true"
console.log(undefined ?? (true ? 'tern-true' : 'tern-false'));

// resolves "tern-false"
console.log(undefined ?? (false ? 'tern-true' : 'tern-false'));

https://jsfiddle.net/hxjzaq5c/1/

I do like the idea of using the default for getNumber, I will switch to that. Thanks for the suggestion

}

// think of singleton behavior but unique to the projectPath
Expand Down Expand Up @@ -243,7 +247,14 @@ export class ShadowRepo {
} catch (e) {
if (e instanceof git.Errors.MultipleGitError) {
this.logger.error('multiple errors on git.add', e.errors.slice(0, 5));
const error = new SfError(e.message, e.name, [], 1);
const error = new SfError(
e.message,
e.name,
[
`This error may be thrown because the number of files that source tracking is batching is exceeding your user specific file limits. Either increase your hard file limit in the same session with 'ulimit -Hn ${this.maxFileAdd}', or set the 'SFDX_SOURCE_TRACKING_BATCH_SIZE' environment variable to a value lower than the output of 'ulimit -Hn'.\nNote: Do set this env var too close or your system will still hit the limit. If you continue to get the error, lower the value of the env var even more.`,
iowillhoit marked this conversation as resolved.
Show resolved Hide resolved
],
1
);
error.setData(e.errors);
throw error;
}
Expand Down