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

Require terraform-docs runs in serial #33

wants to merge 1 commit into
base: master


None yet
1 participant
Copy link

chrisgilmerproj commented Feb 8, 2019

It turns out that pre-commit defaults to running hooks in parallel. This means it will take a batch of files, break them into chunks, and run those together. The script strips the filenames and runs over a set of unique paths. That means that parallel operations can be running int he same directories. To avoid pre-commit doing parallel operations on similar file paths I've added require_serial: true to the hook descriptions.

For some detail, I ran into this while running on docker. I copied a large terraform directory into a docker container and then ran the terraform-docs pre-commit hook to mimic our CI environment in CircleCI. In about 10-20% of cases the hook would fail and the result would be that one or more files were deleted. It turns out the chunking from pre-commit is non-deterministic, so sometimes similar directories would be groups and sometimes not. And when they were not grouped we had a race condition where perl would be operating on an in-place file operation when the underlying file would get removed.

I attempted to revert to using sed and had less failures but it would happen, just less often. I think the combination of perl and docker is what made this show up because perl takes just a bit longer than sed and docker is resource constrained enough on my mac to cause the race condition to bite me.

Fortunately this is a one-line fix and users who are currently experiencing this can immediately fix it by adding the same line, per this example:

  - repo: git://
    rev: v1.8.1
      - id: terraform_docs
        require_serial: true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment