Address pylint "R1732: Consider using 'with' for resource-allocating operations" warnings

Santiago Gilrequested to merge
santigl/pylint-use-with into master

Before raising this MR, consider whether the following are required, and complete if so:

  • [ ] Unit tests
  • [ ] Metrics
  • [ ] Documentation update(s)

If not required, please explain in brief why not.

Description

pylint started to issue warnings suggesting the use context managers in some places (https://gitlab.com/BuildGrid/buildgrid/-/jobs/1212089786). This MR addresses those warnings.

************* Module buildgrid.server._authentication
buildgrid/server/_authentication.py:361:26: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
buildgrid/server/_authentication.py:380:23: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid.server.monitoring
buildgrid/server/monitoring.py:359:16: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid.server.server
buildgrid/server/server.py:188:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
buildgrid/server/server.py:193:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid.server.persistence.sql.impl
buildgrid/server/persistence/sql/impl.py:143:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid.server.cas.storage.disk
buildgrid/server/cas/storage/disk.py:57:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
buildgrid/server/cas/storage/disk.py:70:8: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid.server.cas.storage.with_cache
buildgrid/server/cas/storage/with_cache.py:143:12: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module buildgrid._app.bots.host
buildgrid/_app/bots/host.py:102:8: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

Changes proposed in this merge request:

  • monitoring.py: Use context manager when writing file. (This is the only true positive.)
  • For the rest of the occurrences, allocating a resource that outlives a scope is what we want, so dismiss the suggestion locally by adding # pylint: disable comments. (If false positives keep becoming a problem in the future we can roll back the latter commit and disable R1732 globally.)

Validation

The lint job on the CI passes again.

Merge request reports