Resolve linting issues, consolidate linting makefile targets

[redshiftzero]

Status

Ready for review

Description of Changes

Changes proposed in this pull request:

• Remove make ci-lint job
• Do not run docs linting as part of make lint (takes a while and is annoying when working on code only)
• Fix make lint target fails locally #4423

Testing

• Rebuild your virtualenv, run make lint locally, ensure all passes
• CI lint job passes

Deployment

CI/dev only

Checklist

If you made changes to the server application code:

• Linting (make lint) and tests (make -C securedrop test) pass in the development container

If you made non-trivial code changes:

• I have written a test plan and validated it for this PR

[zenmonkeykstop]

• make lint passes locally
• CI lint passes too, but staging-test-with-rebase does not, as enchant isn't available on the GCE Xenial instance

[heartsucker]

This does not pass locally. Is this meant to be run in the dev container?

find . -name '*.py' | xargs pylint --reports=no --errors-only \
   --disable=no-name-in-module \
   --disable=unexpected-keyword-arg \
   --disable=too-many-function-args \
   --disable=import-error \
   --disable=no-member \
   --max-line-length=100
Using config file /home/heartsucker/code/freedomofpress/securedrop/securedrop/pylintrc
Traceback (most recent call last):
  File "/usr/local/bin/pylint", line 11, in <module>
    sys.exit(run_pylint())
  File "/usr/local/lib/python3.5/dist-packages/pylint/__init__.py", line 16, in run_pylint
    Run(sys.argv[1:])
  File "/usr/local/lib/python3.5/dist-packages/pylint/lint.py", line 1315, in __init__
    linter.load_config_file()
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 678, in load_config_file
    self.global_set_option(option, value)
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 576, in global_set_option
    self._all_options[opt].set_option(opt, value)
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 768, in set_option
    value = _validate(value, optdict, optname)
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 233, in _validate
    return _call_validator(_type, optdict, name, value)
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 214, in _call_validator
    return VALIDATORS[opttype](optdict, option, value)
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 204, in <lambda>
    'choice': lambda opt, name, value: _choice_validator(opt['choices'], name, value),
  File "/usr/local/lib/python3.5/dist-packages/pylint/config.py", line 161, in _choice_validator
    raise optparse.OptionValueError(msg % (name, value, choices))
optparse.OptionValueError: option spelling-dict: invalid value: 'en_US', should be in ['']
Makefile:7: recipe for target 'lint' failed

[rmol]

@heartsucker I think you just need to rebuild your virtualenv. It's missing pyenchant. I also ran into an error during molecule verify -s ansible-config before I ran make clean and bootstrapped it again. Now make lint works for me.

[conorsch]

Rebuild your virtualenv

To accomplish this, I ran pip install -r securedrop/requirements/develop-requirements.txt. Target is now passing locally for me.

As for the CI failure, let's consider baking the enchant apt package into the CI base image, to keep CI runs snappy.

[conorsch]

Built a new CI image (version-controlled outside of this repository), and pushed a small commit to retrigger a build. Let's monitor CI and see how it fares.

[conorsch]

CI failed at

    RUNNING HANDLER [common : Wait for server to come back.] ***********************
    fatal: [app-staging -> localhost]: FAILED! => {"changed": false, "elapsed": 301, "msg": "Timeout when waiting for search string OpenSSH in 192.168.121.100:22"}
    fatal: [mon-staging -> localhost]: FAILED! => {"changed": false, "elapsed": 301, "msg": "Timeout when waiting for search string OpenSSH in 192.168.121.228:22"}

Rerunning to check if flake...

[conorsch]

New CI box is reliably failing when staging VMs come back up after a reboot, as above. That's something we should look into fixing, but to unblock this PR, I'm going to delete the new box and add a task to install enchant inside the CI box on PR run, as originally suggested.

[conorsch]

Update: the naive approach in ae3938f isn't sufficient for the CI environment, specifically because the remote user lacks sudo rights (all SD dev env operations are run as a normal user during the CI job). We can work around that, specifically by using the gcloud tooling directly to run SSH connections, or even to pass in a startup script that will enable us to sideload dependencies. Happy to take a closer look tomorrow, when back at a keyboard.

[codecov-io]

Codecov Report

Merging #4432 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #4432   +/-   ##
========================================
  Coverage    83.72%   83.72%           
========================================
  Files           44       44           
  Lines         2956     2956           
  Branches       321      321           
========================================
  Hits          2475     2475           
  Misses         404      404           
  Partials        77       77

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b1b1ab...45b7139. Read the comment docs.

[kushaldas]

Finally all CI green and it looks good to me. Approved.