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

Handle FileNotFoundError caused by processing new file in progressive mode #2364

Merged

Conversation

kostyaplis
Copy link
Contributor

Fixes #2363

@kostyaplis kostyaplis force-pushed the 2363/fix-progressive-for-new-files branch from 79619d3 to b92ab68 Compare August 30, 2022 05:37
@kostyaplis
Copy link
Contributor Author

kostyaplis commented Aug 30, 2022

Apparently this breaks the desired behaviour when passing a non-existent file to ansible-lint.
Maybe we should be more specific that we are checking the old revision right now, so only in this case we can swallow FileNotFoundError.
E.g.

  1. Add in_previous_revision=False to options Namespace
  2. Set it to True when entering with _previous_revision() and set back to False before exiting
  3. check if getattr(options, "in_previous_revision", False): in _populate_content_cache_from_disk(), then swallow FileNotFoundError

Does this sound reasonable?

@ssbarnea
Copy link
Member

If I understood correctly, you were using progressive mode while still passing filenames as arguments to the linter. TBH, I never considered this use-case as most of the time we call it without even passing any file arguments.

@ssbarnea
Copy link
Member

I personally do not like this hack because it is inside load content, I wonder if we could catch this error in a different place and ignore it.

Do you happen to have a repository where running linter in progressive can reproduce the bug? If so, I may be able to find a better place to catch this error and to ignore it.

@kostyaplis
Copy link
Contributor Author

Yes, you can found test repository here
Run ansible-lint --progressive playbook-test1.yml to reproduce an error.

If I understood correctly, you were using progressive mode while still passing filenames as arguments to the linter. TBH, I never considered this use-case as most of the time we call it without even passing any file arguments.

Our repository contains >400 playbooks and >300 roles, so our idea is to only lint changed/added files and only report regressions(at least for now). That's why all the recent activity around progressive mode, as I'm actively testing it with our use-cases.

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

It seems to be working but because no test was added, we might see it again in the future. Thanks!

@ssbarnea ssbarnea added the bug label Aug 30, 2022
@ssbarnea ssbarnea merged commit 028a2d9 into ansible:main Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progressive mode is failing for new files
2 participants