Merge pull request #1186 from dimaqq/chore-ruff

#1186

What this PR does

In short, we'll be relying on ruff via pre-commit as opposed to flake8 via tox.

- switches from flake8 to ruff
- reformats the source code
- reformats the docstrings where safe, conservatively
- resolves ruff complaints where that's straightforward
- silences with a `FIXME` where public API could break or code change is not obvious or where too much manual work is required, those can be fixed in separate PRs
- updates the CI and docs for the above


Check list

- [x] pre-commit config
- [x] ruff over the codebase
- [x] resolve or silence errors
- [x] kill old linter and github actions
- [x] GitHub actions to check pre-commit
- [x] run pre-commit after codegen
- [x] add to docs

.github/ISSUE_TEMPLATE/BugReport.yml

@@ -53,4 +53,4 @@ body:
         Please provide a simplified reproducer, and if it's possible please refrain from providing a 'clone this repository and run the integration tests to see the problem' type of a reproducer. Thanks!"
       render: python
     validations:
-      required: true
+      required: true

.github/ISSUE_TEMPLATE/FeatureRequest.yml

@@ -33,4 +33,4 @@ body:
       description: "Please provide valid Python code that you'd like to be able to run with this new feature: "
       render: python
     validations:
-      required: true
+      required: true

.github/pull_request_template.md

@@ -23,4 +23,4 @@ All CI tests need to pass.
 
 #### Notes & Discussion
 
-*\<Additional notes for the reviewers if needed. Please delete section if not applicable.\>*
+*\<Additional notes for the reviewers if needed. Please delete section if not applicable.\>*

.github/workflows/pre-commit.yaml

@@ -0,0 +1,17 @@
+---
+name: pre-commit
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-python@v5
+      with:
+        python-version: "3.13"
+    - uses: pre-commit/action@v3.0.1

.github/workflows/static-analysis.yaml

@@ -15,4 +15,3 @@ jobs:
     steps:
         - uses: actions/checkout@v4
         - uses: wagoid/commitlint-github-action@v6
-

.github/workflows/test.yaml

@@ -7,28 +7,6 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  lint:
-    name: Linter
-    runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        python:
-          - "3.9"
-          - "3.10"
-    steps:
-      - name: Check out code
-        uses: actions/checkout@v4
-      - name: Setup Python
-        uses: actions/setup-python@v5
-        with:
-          python-version: ${{ matrix.python }}
-      - name: Install dependencies
-        run: pip install tox
-      - name: Run linter
-        run: |
-          tox -e lint
-          ./scripts/copyright.sh
-
   build-test:
     name: Build test
     runs-on: ubuntu-latest
@@ -37,14 +15,12 @@ jobs:
         python:
           - "3.10"
     steps:
-      - name: Check out code
-        uses: actions/checkout@v4
-      - name: Setup Python
-        uses: actions/setup-python@v5
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python }}
-      - name: Run build test target
-        run: |
+      - uses: astral-sh/setup-uv@v3.2.2
+      - run: |
           make build-test
 
   validate:
@@ -67,7 +43,6 @@ jobs:
         run: tox -e validate
 
   unit-tests:
-    needs: lint
     name: Unit tests
     runs-on: ubuntu-latest
     strategy:
@@ -93,7 +68,7 @@ jobs:
 
   integration:
     name: Integration
-    needs: [lint, unit-tests]
+    needs: [unit-tests]
     timeout-minutes: 150
     runs-on: ubuntu-latest
     strategy:
@@ -161,7 +136,7 @@ jobs:
 
   integration-quarantine:
     name: Quarantined Integration Tests
-    needs: [lint, unit-tests]
+    needs: [unit-tests]
     timeout-minutes: 150
     runs-on: ubuntu-latest
     strategy:

.github/workflows/test_candidate.yaml

@@ -42,7 +42,7 @@ jobs:
             else
               echo "Candidate $candidate has to be tested"
               next_test="$candidate"
-            fi 
+            fi
           fi
           echo "next-test=$next_test" >> $GITHUB_ENV
           echo "$next_test" > ~/juju-last-candidate-version

.github/workflows/test_edge.yaml

@@ -42,7 +42,7 @@ jobs:
             else
               echo "Edge $edge has to be tested"
               next_test="$edge"
-            fi 
+            fi
           fi
           echo "next-test=$next_test" >> $GITHUB_ENV
           echo "$next_test" > ~/juju-last-edge-version

.pre-commit-config.yaml

@@ -0,0 +1,46 @@
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v4.5.0
+    hooks:
+      - id: check-added-large-files
+        exclude: |
+          (?x)  # Verbose mode
+          ^juju/client/schemas-juju-.*[.]json$  |
+          ^tests/.*[.]charm$  |
+          ^examples/.*[.]charm$
+      - id: check-ast
+      - id: check-case-conflict
+      - id: check-executables-have-shebangs
+      - id: check-shebang-scripts-are-executable
+      - id: check-merge-conflict
+      - id: check-symlinks
+      - id: check-json
+      - id: check-yaml
+        # Overlays deliberately comprise multiple YAML documents per file
+        exclude: "^tests/integration/bundle/test-overlays/.*multi.*yaml$"
+      - id: check-toml
+      - id: mixed-line-ending
+      - id: end-of-file-fixer
+        exclude: "^juju/client/schemas-juju-.*[.]json$"
+      - id: trailing-whitespace
+      - id: detect-private-key
+        exclude: "^tests/.*$"
+
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.7.3
+    hooks:
+      - id: ruff
+        args: [ --preview, --fix ]
+      - id: ruff-format
+        args: [ --preview ]
+
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.3.0
+    hooks:
+    - id: codespell
+      additional_dependencies:
+        - tomli
+      exclude: "^juju/client/schemas-juju-.*[.]json$"
+
+ci:
+  autofix_prs: false

.readthedocs.yaml

@@ -10,4 +10,4 @@ build:
     python: "3.10"
 
 sphinx:
-  configuration: docs/conf.py
+  configuration: docs/conf.py

Makefile

@@ -16,47 +16,42 @@ clean:
 
 .PHONY: client
 client:
-	tox -r --notest -e lint,py3
+	tox -r --notest -e py3
 	$(PY) -m juju.client.facade -s "juju/client/schemas*" -o juju/client/
+	pre-commit run --files $(shell echo juju/client/_[cd]*.py)
 
 .PHONY: run-unit-tests
-run-unit-tests: .tox lint
+run-unit-tests: .tox
 	tox -e py3
 
 .PHONY: run-integration-tests
-run-integration-tests: .tox lint
+run-integration-tests: .tox
 	tox -e integration
 
 .PHONY: run-all-tests
 test: run-unit-tests run-integration-tests
 
-.PHONY: lint
-lint:
-	@./scripts/copyright.sh
-	@echo "==> Running flake8 linter"
-	tox -e lint
-
 .PHONY: docs
 docs:
 	tox -e docs
 
 .PHONY: build-test
 build-test:
 	rm -rf venv
+	uv build
 	python3 -m venv venv
 	. venv/bin/activate
-	python3 setup.py sdist
-	pip install ./dist/juju-${VERSION}.tar.gz
+	pip install dist/juju-${VERSION}-py3-none-any.whl
 	python3 -c "from juju.controller import Controller"
-	rm ./dist/juju-${VERSION}.tar.gz
+	rm dist/*.tar.gz dist/*.whl
 
 .PHONY: release
 release:
 	git fetch --tags
-	rm dist/*.tar.gz || true
-	$(PY) setup.py sdist
-	$(BIN)/twine check dist/*
-	$(BIN)/twine upload --repository juju dist/*
+	rm dist/*.tar.gz dist/*.whl || true
+	uv build
+	uvx twine check dist/*
+	uvx twine upload --repository juju dist/*
 	git tag ${VERSION}
 	git push --tags
 

debian/changelog

@@ -17,7 +17,7 @@ Thursday January 9 2020
 Tuesday January 7 2020
 
  * Update facade methods for Juju 2.7.0
- * Fix an issue when querying CMR relations (#366) 
+ * Fix an issue when querying CMR relations (#366)
  * Fix storage support in bundles (#361)
  * Fix reporting of unit leaders (#374)
  * AddCloud API support (#370)

debian/control

@@ -130,4 +130,3 @@ Description:
     .
     Pylibjuju releases now track the Juju release cadence. New generated schemas
     will be updated per Juju releases.
-

debian/copyright

@@ -212,4 +212,3 @@ License: Apache-2
    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    See the License for the specific language governing permissions and
    limitations under the License.
-

docs/CONTRIBUTING.md

@@ -29,6 +29,12 @@ Check to see what [types of contributions](/contributing/types-of-contributions.
 -->
 
 
+## Tooling
+
+- `uv`, https://docs.astral.sh/uv/
+- `pre-commit`, https://pre-commit.com/
+- Python 3.8 ~ 3.13, https://www.python.org/downloads/
+
 ## Pull Request
 
 Thanks for considering to contribute code to our project! We accept and welcome all kinds of PRs! :tada:
@@ -40,6 +46,7 @@ The process is very simple.
 - Check with `git remote -v`, origin should point to your fork, upstream should point to ours.
 - If your changes are going to be on top of a specific branch (e.g., `2.9`), then fetch and switch to that.
 - `git switch -c your-branch-name`
+- Make sure that you have run `pre-commit install`
 - Make your changes, create your commits.
 - `git push -u origin your-branch-name`
 

docs/_extensions/automembersummary.py

@@ -33,45 +33,46 @@ def run(self):
         try:
             module = importlib.import_module(module_name)
         except ImportError:
-            raise SphinxError("Unable to generate reference docs for %s, "
-                              "could not import" % (module_name))
+            raise SphinxError(
+                "Unable to generate reference docs for %s, "
+                "could not import" % (module_name)
+            )
 
-        divider = '+{:-<80}+'.format('')
-        row = '| {:<78} |'.format
+        divider = "+{:-<80}+".format("")
+        row = "| {:<78} |".format
         lines = []
         for member_name, member in inspect.getmembers(module):
             if not self._filter(module_name, member_name, member):
                 continue
-            summary = textwrap.wrap(self._get_summary(member), 78) or ['']
-            link = '`{} <#{}>`_'.format(member_name,
-                                        '.'.join([module_name,
-                                                  member_name]))
-            methods = ['* `{} <#{}>`_'.format(n,
-                                              '.'.join([module_name,
-                                                        member_name,
-                                                        n]))
-                       for n, m in inspect.getmembers(member)
-                       if not n.startswith('_') and inspect.isfunction(m)]
+            summary = textwrap.wrap(self._get_summary(member), 78) or [""]
+            link = "`{} <#{}>`_".format(
+                member_name, ".".join([module_name, member_name])
+            )
+            methods = [
+                "* `{} <#{}>`_".format(n, ".".join([module_name, member_name, n]))
+                for n, m in inspect.getmembers(member)
+                if not n.startswith("_") and inspect.isfunction(m)
+            ]
 
             lines.append(divider)
             lines.append(row(link))
             lines.append(divider)
             for line in summary:
-                lines.append(row(line))
+                lines.append(row(line))  # noqa: PERF401
             if methods:
-                lines.append(row(''))
-                lines.append(row('Methods:'))
-                lines.append(row(''))
-                for i, method in enumerate(methods):
+                lines.append(row(""))
+                lines.append(row("Methods:"))
+                lines.append(row(""))
+                for _, method in enumerate(methods):
                     lines.append(row(method))
         lines.append(divider)
-        content = '\n'.join(lines)
+        content = "\n".join(lines)
 
-        result = self._parse(content, '<automembersummary>')
+        result = self._parse(content, "<automembersummary>")
         return result
 
     def _get_summary(self, member):
-        doc = (member.__doc__ or '').splitlines()
+        doc = (member.__doc__ or "").splitlines()
 
         # strip any leading blank lines
         while doc and not doc[0].strip():
@@ -86,12 +87,12 @@ def _get_summary(self, member):
         return " ".join(doc).strip()
 
     def _filter(self, module_name, member_name, member):
-        if member_name.startswith('_'):
+        if member_name.startswith("_"):
             return False
-        if hasattr(member, '__module__'):
+        if hasattr(member, "__module__"):
             # skip imported classes & functions
             return member.__module__.startswith(module_name)
-        elif hasattr(member, '__name__'):
+        elif hasattr(member, "__name__"):
             # skip imported modules
             return member.__name__.startswith(module_name)
         else:
@@ -109,4 +110,4 @@ def _parse(self, rst_text, annotation):
 
 
 def setup(app):
-    app.add_directive('automembersummary', AutoMemberSummary)
+    app.add_directive("automembersummary", AutoMemberSummary)