From eb211431b3d7ebf3d61bc2f1f79f52e137003943 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Tue, 6 Aug 2024 07:36:04 +0100 Subject: [PATCH 1/5] run safe ruff fixes --- utils/communication_utils/channel_access.py | 2 +- utils/hotfix_utils/RepoChecker.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/communication_utils/channel_access.py b/utils/communication_utils/channel_access.py index e43cf32..eb3f077 100644 --- a/utils/communication_utils/channel_access.py +++ b/utils/communication_utils/channel_access.py @@ -9,7 +9,7 @@ from enum import ( Enum, ) -from typing import Dict, Any +from typing import Any, Dict from genie_python.channel_access_exceptions import ( ReadAccessException, diff --git a/utils/hotfix_utils/RepoChecker.py b/utils/hotfix_utils/RepoChecker.py index a63caf0..45dc0aa 100644 --- a/utils/hotfix_utils/RepoChecker.py +++ b/utils/hotfix_utils/RepoChecker.py @@ -4,7 +4,7 @@ import sys import requests -from packaging.version import Version, InvalidVersion +from packaging.version import InvalidVersion, Version from utils.hotfix_utils.InstrumentChecker import InstrumentChecker From 2c039635bbef50f058e9d05cb863104f8dd56509 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Tue, 6 Aug 2024 07:36:05 +0100 Subject: [PATCH 2/5] run ruff format --- hotfix_checker.py | 4 +- utils/communication_utils/channel_access.py | 6 +-- utils/communication_utils/ssh_access.py | 1 + utils/hotfix_utils/InstrumentChecker.py | 6 ++- utils/hotfix_utils/RepoChecker.py | 45 ++++++++++++++------- utils/jenkins_utils/jenkins_utils.py | 4 +- 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/hotfix_checker.py b/hotfix_checker.py index e417d6f..753c1e4 100644 --- a/hotfix_checker.py +++ b/hotfix_checker.py @@ -18,9 +18,7 @@ print(f"INFO: REPO_DIR: {os.environ['REPO_DIR']}") print(f"INFO: UPSTREAM_BRANCH: {os.environ['UPSTREAM_BRANCH_CONFIG']}") print(f"INFO: ARTEFACT_DIR: {os.environ['WORKSPACE']}") - print( - f"INFO: USE_TEST_INSTRUMENT_LIST: {os.environ['USE_TEST_INSTRUMENT_LIST']}" - ) + print(f"INFO: USE_TEST_INSTRUMENT_LIST: {os.environ['USE_TEST_INSTRUMENT_LIST']}") print(f"INFO: TEST_INSTRUMENT_LIST: {os.environ['TEST_INSTRUMENT_LIST']}") print(f"INFO: DEBUG_MODE: {os.environ['DEBUG_MODE']}") diff --git a/utils/communication_utils/channel_access.py b/utils/communication_utils/channel_access.py index eb3f077..ce59653 100644 --- a/utils/communication_utils/channel_access.py +++ b/utils/communication_utils/channel_access.py @@ -36,7 +36,7 @@ def __init__( def get_value( self, pv, - ) -> str | dict | None: + ) -> str | dict | None: """Gets the value of the PV. Returns None if PV is unavailable. :return: The PV value as a string, or None if there was an error. """ @@ -72,9 +72,7 @@ def get_inst_list( :return: a list of strings of instrument names. """ pv_value = self.get_value("CS:INSTLIST") - return ( - {} if pv_value is None else json.loads(self._dehex_and_decompress(pv_value)) - ) + return {} if pv_value is None else json.loads(self._dehex_and_decompress(pv_value)) class PvInterestingLevel(Enum): diff --git a/utils/communication_utils/ssh_access.py b/utils/communication_utils/ssh_access.py index 6e6a6d1..78d7ac6 100644 --- a/utils/communication_utils/ssh_access.py +++ b/utils/communication_utils/ssh_access.py @@ -1,4 +1,5 @@ """This module provides utilities for SSH access.""" + from typing import Dict import paramiko diff --git a/utils/hotfix_utils/InstrumentChecker.py b/utils/hotfix_utils/InstrumentChecker.py index 253c4ec..e00ea29 100644 --- a/utils/hotfix_utils/InstrumentChecker.py +++ b/utils/hotfix_utils/InstrumentChecker.py @@ -207,7 +207,7 @@ def split_git_log(self, git_log: str, prefix: str) -> dict: if prefix is None or message.startswith(prefix): commit_dict[hash] = message return commit_dict - + def check_instrument(self) -> dict: """Check if there are any hotfixes or uncommitted changes on AN instrument. @@ -257,7 +257,9 @@ def check_instrument(self) -> dict: ) # Check if any uncommitted changes are on the instrument - self.uncommitted_changes_enum, self.uncommitted_changes_messages = self.check_for_uncommitted_changes() + self.uncommitted_changes_enum, self.uncommitted_changes_messages = ( + self.check_for_uncommitted_changes() + ) def as_string(self) -> str: """Return the Instrument object as a string. diff --git a/utils/hotfix_utils/RepoChecker.py b/utils/hotfix_utils/RepoChecker.py index 45dc0aa..a49773e 100644 --- a/utils/hotfix_utils/RepoChecker.py +++ b/utils/hotfix_utils/RepoChecker.py @@ -67,15 +67,15 @@ def get_insts_on_latest_ibex_via_inst_config(self) -> list: latest_major_version = versions[-1].major second_latest_major_version = latest_major_version - 1 - print(f"INFO: checking versions {latest_major_version}.x.x and {second_latest_major_version}.x.x") + print( + f"INFO: checking versions {latest_major_version}.x.x and {second_latest_major_version}.x.x" + ) # filter out the instruments that are not on the latest version insts_on_latest_ibex = [ inst["hostname"] for inst in result_list - if ( - inst["version"].major in [latest_major_version, second_latest_major_version] - ) + if (inst["version"].major in [latest_major_version, second_latest_major_version]) ] return insts_on_latest_ibex @@ -105,11 +105,10 @@ def check_instruments(self) -> None: def update_instrument_status_lists(instrument, status_list_key, messages=None): if messages: - instrument_status_lists[status_list_key].append({instrument.hostname : messages}) + instrument_status_lists[status_list_key].append({instrument.hostname: messages}) else: instrument_status_lists[status_list_key].append(instrument.hostname) - for hostname in instrument_list: instrument = InstrumentChecker(hostname) try: @@ -119,17 +118,34 @@ def update_instrument_status_lists(instrument, status_list_key, messages=None): print(instrument.as_string()) if instrument.commits_local_not_on_upstream_enum == CHECK.TRUE: - update_instrument_status_lists(instrument, self._commits_on_local_not_upstream_key, instrument.commits_local_not_on_upstream_messages) - + update_instrument_status_lists( + instrument, + self._commits_on_local_not_upstream_key, + instrument.commits_local_not_on_upstream_messages, + ) + if instrument.uncommitted_changes_enum == CHECK.TRUE: - update_instrument_status_lists(instrument, self._uncommitted_changes_key, instrument.uncommitted_changes_messages) - + update_instrument_status_lists( + instrument, + self._uncommitted_changes_key, + instrument.uncommitted_changes_messages, + ) if instrument.commits_upstream_not_on_local_enum == CHECK.TRUE: - update_instrument_status_lists(instrument, self._commits_on_upstream_not_local_key, instrument.commits_upstream_not_on_local_messages) + update_instrument_status_lists( + instrument, + self._commits_on_upstream_not_local_key, + instrument.commits_upstream_not_on_local_messages, + ) - if instrument.commits_local_not_on_upstream_enum == CHECK.UNDETERMINABLE or instrument.uncommitted_changes_enum == CHECK.UNDETERMINABLE or instrument.commits_upstream_not_on_local_enum == CHECK.UNDETERMINABLE: - update_instrument_status_lists(instrument, self._undeterminable_at_some_point_key) + if ( + instrument.commits_local_not_on_upstream_enum == CHECK.UNDETERMINABLE + or instrument.uncommitted_changes_enum == CHECK.UNDETERMINABLE + or instrument.commits_upstream_not_on_local_enum == CHECK.UNDETERMINABLE + ): + update_instrument_status_lists( + instrument, self._undeterminable_at_some_point_key + ) except Exception as e: print(f"ERROR: Could not connect to {instrument.hostname} ({str(e)})") @@ -140,8 +156,7 @@ def update_instrument_status_lists(instrument, status_list_key, messages=None): (self._commits_on_local_not_upstream_key, "Commits on local not upstream"), (self._commits_on_upstream_not_local_key, "Commits on upstream not on local"), (self._undeterminable_at_some_point_key, "Undeterminable at some point"), -] - + ] print("INFO: Summary of results") for key, prefix in keys_and_prefixes: diff --git a/utils/jenkins_utils/jenkins_utils.py b/utils/jenkins_utils/jenkins_utils.py index 98724ed..ef699ae 100644 --- a/utils/jenkins_utils/jenkins_utils.py +++ b/utils/jenkins_utils/jenkins_utils.py @@ -24,9 +24,7 @@ def save_git_status( """ # log the output to a workspace file for viewing later - if not os.path.exists( - os.path.join(artefact_dir, "git_status") - ): + if not os.path.exists(os.path.join(artefact_dir, "git_status")): os.makedirs(os.path.join(artefact_dir, "git_status")) with open( From e4b1b876cdbf7f7c4a0bf28477fdebcbde31fc65 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Tue, 6 Aug 2024 07:36:11 +0100 Subject: [PATCH 3/5] adding ruff check and format to blame ignore, add linter --- .git-blame-ignore-revs | 2 ++ .github/workflows/linters.yml | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 .git-blame-ignore-revs create mode 100644 .github/workflows/linters.yml diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 0000000..1453628 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,2 @@ +eb211431b3d7ebf3d61bc2f1f79f52e137003943 +2c039635bbef50f058e9d05cb863104f8dd56509 diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml new file mode 100644 index 0000000..2b10d7d --- /dev/null +++ b/.github/workflows/linters.yml @@ -0,0 +1,66 @@ +name: Linters +on: + workflow_call: + inputs: + compare-branch: + required: true + type: string + python-ver: + required: false + type: string +jobs: + ruff: + runs-on: ubuntu-latest + env: + CONFIG: .ibex-shared-workflows/ruff.toml + steps: + - name: get Python version or use default. + run: | + echo "PY_VER=${{ inputs.python-ver || '3.10' }}" >> $GITHUB_ENV + - uses: actions/setup-python@v5 + with: + python-version: ${{ env.PY_VER }} + - run: pip install ruff + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + # If no local ruff.toml exists fetch the default config from reusable workflows. + - name: Use default config + uses: actions/checkout@v4 + if: ${{ hashFiles('ruff.toml') == ''}} + with: + repository: ISISComputingGroup/reusable-workflows + path: .ibex-shared-workflows + # If a local config exists set the config to be the environment variable so it is properly used. + - name: Use local config. + if: ${{ hashFiles('ruff.toml') != ''}} + run: | + echo "CONFIG=ruff.toml" >> $GITHUB_ENV + - name: Changed Files + run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty echo + - name: Run Ruff Check on Changed Files + run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty ruff check --config ${{ env.CONFIG }} --output-format=github + - name: Run Ruff Format Check on Changed Files + if: success() || failure() # Run the format check even if the previous check fails (but not always, as should not run on a failed build.) + run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty ruff format --check --config ${{ env.CONFIG }} + pyright: + runs-on: ubuntu-latest + steps: + - name: get Python version or use default. + run: | + echo "PY_VER=${{ inputs.python-ver || '3.10' }}" >> $GITHUB_ENV + - uses: actions/setup-python@v5 + with: + python-version: ${{ env.PY_VER }} + - run: pip install unittest-xml-reporting + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: install pyright + run: pip install pyright + - name: install diff_cover + run: pip install diff_cover + - name: install pyright_plugin + run: pip install git+https://github.com/DiamondLightSource/pyright_diff_quality_plugin.git + - name: Run Pyright on changes + run: diff-quality --violations=pyright --fail-under=100 --compare-branch ${{ inputs.compare-branch }} From dabbf30488141773aa0ff3ccddf7d57d0f7471f7 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Tue, 6 Aug 2024 07:42:06 +0100 Subject: [PATCH 4/5] adding ruff check and format to blame ignore, add linter --- .github/workflows/linters.yml | 71 +++-------------------------------- 1 file changed, 6 insertions(+), 65 deletions(-) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index 2b10d7d..72d9425 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -1,66 +1,7 @@ -name: Linters -on: - workflow_call: - inputs: - compare-branch: - required: true - type: string - python-ver: - required: false - type: string +name: Linter +on: [pull_request] jobs: - ruff: - runs-on: ubuntu-latest - env: - CONFIG: .ibex-shared-workflows/ruff.toml - steps: - - name: get Python version or use default. - run: | - echo "PY_VER=${{ inputs.python-ver || '3.10' }}" >> $GITHUB_ENV - - uses: actions/setup-python@v5 - with: - python-version: ${{ env.PY_VER }} - - run: pip install ruff - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - # If no local ruff.toml exists fetch the default config from reusable workflows. - - name: Use default config - uses: actions/checkout@v4 - if: ${{ hashFiles('ruff.toml') == ''}} - with: - repository: ISISComputingGroup/reusable-workflows - path: .ibex-shared-workflows - # If a local config exists set the config to be the environment variable so it is properly used. - - name: Use local config. - if: ${{ hashFiles('ruff.toml') != ''}} - run: | - echo "CONFIG=ruff.toml" >> $GITHUB_ENV - - name: Changed Files - run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty echo - - name: Run Ruff Check on Changed Files - run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty ruff check --config ${{ env.CONFIG }} --output-format=github - - name: Run Ruff Format Check on Changed Files - if: success() || failure() # Run the format check even if the previous check fails (but not always, as should not run on a failed build.) - run: git diff ${{ inputs.compare-branch }}..HEAD --name-only --diff-filter=ACM -z "*.py" | xargs -0 --no-run-if-empty ruff format --check --config ${{ env.CONFIG }} - pyright: - runs-on: ubuntu-latest - steps: - - name: get Python version or use default. - run: | - echo "PY_VER=${{ inputs.python-ver || '3.10' }}" >> $GITHUB_ENV - - uses: actions/setup-python@v5 - with: - python-version: ${{ env.PY_VER }} - - run: pip install unittest-xml-reporting - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: install pyright - run: pip install pyright - - name: install diff_cover - run: pip install diff_cover - - name: install pyright_plugin - run: pip install git+https://github.com/DiamondLightSource/pyright_diff_quality_plugin.git - - name: Run Pyright on changes - run: diff-quality --violations=pyright --fail-under=100 --compare-branch ${{ inputs.compare-branch }} + call-workflow: + uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main + with: + compare-branch: origin/main From 83e6471ff2e9af85f66df5b4f5be2adc3558d299 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Tue, 6 Aug 2024 07:43:06 +0100 Subject: [PATCH 5/5] use master not main --- .github/workflows/linters.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml index 72d9425..82ad245 100644 --- a/.github/workflows/linters.yml +++ b/.github/workflows/linters.yml @@ -4,4 +4,4 @@ jobs: call-workflow: uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main with: - compare-branch: origin/main + compare-branch: origin/master