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

Ensure exclude_paths works even if files are provided on the command line #72

Closed
myii opened this issue Oct 22, 2019 · 5 comments
Closed
Assignees
Labels

Comments

@myii
Copy link
Contributor

myii commented Oct 22, 2019

Based upon this configuration in the php-formula:

exclude_paths:
  # Save time spent checking files by avoiding the deprecated `ng/` directory
  - php/ng/

I've found that there is still a significant delay when salt-lint is supplied the files under this directory via. the command line:

$ php-formula$ git ls-files | grep php/ng/ | grep '\.sls$\|\.jinja$\|\.j2$\|\.tmpl$' | xargs -I {} salt-lint {}

The delay in this case is an extra 2 minutes during our Travis run (3 minutes for the php-formula compared to 1 minute for the other formulas).


Taking an example from yamllint. Assuming there's a violation in openvpn/defaults.yaml:

openvpn-formula$ yamllint -s openvpn/defaults.yaml 
openvpn/defaults.yaml
  19:16     warning  too few spaces before comment  (comments)

If the file is ignored in the config:

ignore: |
  openvpn/defaults.yaml

Then it will not run a check for it:

openvpn-formula$ yamllint -s openvpn/defaults.yaml 

The idea is for salt-lint to skip these files, in all cases, even if they are specified on the command line.

@matthijswendelaar
Copy link
Contributor

@myii salt-lint already skips these files even though they are provided on the command line.

The current command:

git ls-files | grep php/ng/ | grep '\.sls$\|\.jinja$\|\.j2$\|\.tmpl$' | xargs -I {} salt-lint {}

Takes around 17 seconds to finish

15.06s user 1.65s system 99% cpu 16.723 total

Improving the command:

git ls-files | grep php/ng/ | grep '\.sls$\|\.jinja$\|\.j2$\|\.tmpl$' | xargs salt-lint

Takes around 0.2 seconds to finish

0.16s user 0.02s system 98% cpu 0.180 total

So it looks likes the command that you are using is causing the delay.
This is because with the first command, salt-lint is called for every file that exists.
If there are 100 files in the directory, salt-lint will be called a 100 times with 1 argument.

The second command is slightly different. It calls salt-lint once, with all the files as arguments.

@myii
Copy link
Contributor Author

myii commented Nov 1, 2019

Good catch @matthijswendelaar, I'll check this out. This invocation was shared, probably a remnant from a previous linter:

@matthijswendelaar
Copy link
Contributor

If it solves the problem, let me know.

@myii
Copy link
Contributor Author

myii commented Nov 1, 2019

Will do, if all is OK, I'll close this issue as well. Thanks again.

@myii
Copy link
Contributor Author

myii commented Nov 1, 2019

@matthijswendelaar Checked this while reviewing #90, everything appears fine, thanks. I'm closing this issue.

CC: @sblaisot -- you probably want to update your salt-lint invocation as well.

@myii myii closed this as completed Nov 1, 2019
myii pushed a commit to myii/ssf-formula that referenced this issue Nov 2, 2019
# [1.45.0](v1.44.1...v1.45.0) (2019-11-02)

### Features

* **salt-lint:** improve `salt-lint` invocation (better performance) ([ecc81b0](ecc81b0)), closes [/github.com/warpnet/salt-lint/issues/72#issuecomment-548738115](https://github.com//github.com/warpnet/salt-lint/issues/72/issues/issuecomment-548738115)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants