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.

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

perf(sam): do not watch files excluded in user settings #3770

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Conversation

justinmk3
Copy link
Contributor

@justinmk3 justinmk3 commented Aug 24, 2023

Problem

Solution

  • When scanning for CFN/SAM templates, exclude all directories/files specified in all of the vscode settings files.exclude, search.exclude, or files.watcherExclude.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@justinmk3 justinmk3 requested a review from a team as a code owner August 24, 2023 15:56
@justinmk3 justinmk3 changed the title Samscan perf(sam): do not watch files excluded in user settings Aug 24, 2023
@justinmk3
Copy link
Contributor Author

/runintegrationtests

Problem:
"Scanning CloudFormation templates..." takes a long time on big
workspaces, and there is no workaround. #3510

Solution:
When scanning for CFN/SAM templates, exclude all directories/files
specified in _all_ of the vscode settings `files.exclude`,
`search.exclude`, or `files.watcherExclude`.
Problem:
vscode extension host logs show many warnings like this:

    [warning] [amazonwebservices.aws-toolkit-vscode] Accessing a resource scoped configuration without providing a resource is not expected.
    To get the effective value for 'files.watcherExclude', provide the URI of a resource or 'null' for any resource

Solution:
Default to `null` instead of `undefined` when getting a config.
Problem:
If a setting value is invalid, `Settings.instance.get` throws an
exception, even if a `defaultValue` is provided. We almost never want
that behavior. Example:

    Settings.instance.get<object>('files.exclude', Number, 42)

Solution:
Copy the logic from `AnonymousSettings.get()`.
@justinmk3 justinmk3 merged commit 4bdd722 into master Aug 24, 2023
@justinmk3 justinmk3 deleted the samscan branch August 24, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant