search

fermitools / jobsub_lite

jobsub_lite is a wrapper for HTCondor job submission
Apache License 2.0
1 stars 7 forks source link

pylint cleanup #513

Closed shreyb closed 9 months ago

shreyb commented 10 months ago

It turns out that our pre-commit config was actually not running the pylint tests. I fixed that here. Here are the major changes in this PR:

  1. Change mentioned above
  2. Most changes are import-level changes (deleting unused imports or reordering them).
  3. A couple of changes suggested by pylint, along with disabling some checks on a line/function basis so we can come back to them. The code works now, and this PR shouldn't break anything.
  4. PR #508 introduced cyclic imports in lib/condor.py that both the author and reviewer missed. Unit and integration tests passed, so the fact that pylint wasn't working before meant that this was missed when development on that PR was being done. We need to decide whether to fix that issue in this PR, or open a new issue for that. If the latter, we should definitely have it fixed before the next release.
shreyb commented 10 months ago

Before anyone reviews, I'll check to see why the CI tests failed and try to resolve those.

shreyb commented 10 months ago

So unfortunately, according to the docs here: https://pylint.pycqa.org/en/latest/user_guide/installation/pre-commit-integration.html, pylint has to be run locally (not inside an isolated environment as pre-commit CI configuration would normally do). As such, I've made it so that the pylint tests work if a developer has installed pre-commit, but are skipped for CI, which they already were. I think as a separate issue, I'll get a github actions container spun up where we can, among unit tests, run pylint on the code base.