Add LOG_FILE_APPEND to settings

[erikkemperman]

Hi,

First of all, thanks for a great tool!

I wanted to truncate my logfile at every new run, but noticed that the logfile is opened with mode='a' and there's no way to override that. So I made a PR which allows us to pass a LOG_FILE_TRUNCATE setting, defaulting to False. If it's set to True, the logfile will be opened with mode='w'. The setting has no effect unless LOG_FILE is set, obviously.

Hopefully I've added sufficient info to the docs to explain this item.

I've also added a test, although it seems slightly out of place in test_crawler.py -- I put it there because it's the only file in the tests directory where I found an occurrence of LOG_FILE.

[Gallaecio]

Nice work!

[erikkemperman]

Changed to APPEND. I'm baffled why it says here I started a review, certainly that wasn't my intention :-)

[erikkemperman]

(Sorry, I forgot to change the commit message)

[Gallaecio]

Don’t worry about keeping the commit history clean, we will probably squash your changes into a single commit upon merging anyway.

[codecov]

Codecov Report

Merging #5279 (b85207e) into master (5b13bfd) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head b85207e differs from pull request most recent head d774d6a. Consider uploading reports for the commit d774d6a to get more accurate results

@@            Coverage Diff             @@
##           master    #5279      +/-   ##
==========================================
- Coverage   88.52%   88.49%   -0.03%     
==========================================
  Files         163      163              
  Lines       10608    10608              
  Branches     1557     1527      -30     
==========================================
- Hits         9391     9388       -3     
- Misses        942      945       +3     
  Partials      275      275              

Impacted Files	Coverage Δ
scrapy/settings/default_settings.py	98.75% <100.00%> (+<0.01%)	⬆️
scrapy/utils/log.py	89.36% <100.00%> (+0.11%)	⬆️
scrapy/downloadermiddlewares/cookies.py	94.80% <0.00%> (-2.60%)	⬇️
scrapy/shell.py	67.96% <0.00%> (-0.79%)	⬇️
scrapy/http/response/__init__.py	97.43% <0.00%> (-0.04%)	⬇️
scrapy/http/request/__init__.py	97.77% <0.00%> (-0.03%)	⬇️
scrapy/utils/python.py	87.64% <0.00%> (ø)
scrapy/commands/check.py	71.01% <0.00%> (ø)

[erikkemperman]

Ah, sloppy/paste in the test function: thanks for removing the unused variable!

[erikkemperman]

Thanks for review and merge!

By the way, I looked into the problem with recent flake8 versions -- i.e., the reason why you had to add 98ee3dd.

I've submitted a PR which would, I think, resolve the issue here:
tholo/pytest-flake8#82

If that were to be accepted, merged and released, then scrapy could drop this version pin and use latest flake8 releases.

[Gallaecio]

That’s awesome. However, freezing these static checks, and upgrading them manually regularly (e.g. after a release) is something we are actually moving towards. Even if bug free, new versions of checkers may introduce new checks that our code does not pass in master, and having pull requests fail in CI because of static check tool upgrades can be confusing for contributors.

[erikkemperman]

Fair enough -- but at least you'll have the option of pinning it to the current major release, if you wanted to. (Well, if that PR is accepted, anyway).