Skip to content

fix: Recreate SnmpEngine after each call.#2

Merged
abaczek merged 2 commits intomainfrom
snmp
Jan 8, 2026
Merged

fix: Recreate SnmpEngine after each call.#2
abaczek merged 2 commits intomainfrom
snmp

Conversation

@mchromin
Copy link
Copy Markdown
Contributor

@mchromin mchromin commented Jan 2, 2026

  • async.run triggers new event loop with each call
  • SnmpEngine keeps some reference to the first passed event loop, so to keep this object cached we should fully switch to async in this module
  • as there is not much of an overhead to recreate SnmpEngine object and this PDU calls are not focused on best possible performance it is easier to recreate engine with each call. If one day we will need to boost performance of this operations we can come back to switching to async

Signed-off-by: Mateusz Chrominski <mateusz.chrominski@intel.com>
Copilot AI review requested due to automatic review settings January 2, 2026 13:56
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an event loop compatibility issue with SnmpEngine by recreating it for each SNMP call instead of caching it as an instance variable. The change ensures proper functioning when asyncio.run() creates a new event loop with each invocation. Additionally, the PR significantly refactors GitHub Actions workflows to use centralized reusable workflows from the intel/mfd repository.

Key changes:

  • Remove cached _snmp_engine instance variable and create a new SnmpEngine() for each SNMP operation
  • Replace custom workflow definitions with calls to centralized reusable workflows from intel/mfd/.github/workflows/
  • Add new dependency management configurations (dependabot.yml and dependency_review.yml)

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mfd_powermanagement/pdu.py Removes cached SnmpEngine instance; creates new instance per call to fix event loop compatibility
.github/workflows/run_tests.yml New workflow that delegates test execution to centralized workflow
.github/workflows/pull_requests.yml Deleted - functionality moved to pull_request.yml
.github/workflows/pull_request.yml New workflow for PR builds using centralized workflow
.github/workflows/manual_release.yml Simplified to use centralized release workflow with updated matrix configuration
.github/workflows/main.yml New workflow for main branch builds using centralized workflow
.github/workflows/dependency_review.yml New workflow for dependency review using centralized workflow
.github/workflows/codeql.yml Simplified to use centralized CodeQL workflow
.github/workflows/check_pr_format.yml New workflow for PR title and commit validation
.github/workflows/check_code_standard.yml New workflow for code standard checks
.github/workflows/build_upload_whl.yml Deleted - functionality moved to centralized workflows
.github/dependency_review.yml New configuration for dependency review settings
.github/dependabot.yml New configuration for automated dependency updates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +9 to +17
strategy:
fail-fast: false
matrix:
python_version: ['3.10', '3.13']
uses: intel/mfd/.github/workflows/check_code_standard.yml@main
with:
REPOSITORY_NAME: ${{ github.event.pull_request.head.repo.full_name }}
BRANCH_NAME: ${{ github.head_ref }}
PYTHON_VERSION: ${{ matrix.python_version }} No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

In general, the fix is to explicitly declare a permissions: block in the workflow, restricting the GITHUB_TOKEN to the minimum privileges needed. Since this workflow only runs on pull_request and delegates to a reusable workflow that presumably reads code and PR metadata, a safe and minimal set is to grant contents: read and pull-requests: read. This prevents unintended write access by default while still allowing the workflow to inspect the repository contents and pull request information.

The best way to fix this specific file without changing existing functionality is to add a top‑level permissions: section (applies to all jobs) just after the on: block and before jobs:. This will ensure all jobs in this workflow, including run_check_standard, run with a read‑only token appropriate for checks. No imports or additional methods are needed, as this is purely a YAML configuration change in .github/workflows/check_code_standard.yml.

Concretely:

  • Edit .github/workflows/check_code_standard.yml.
  • Insert a new permissions: block at the root level (same indentation as on: and jobs:).
  • Set:
    • contents: read
    • pull-requests: read
Suggested changeset 1
.github/workflows/check_code_standard.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/check_code_standard.yml b/.github/workflows/check_code_standard.yml
--- a/.github/workflows/check_code_standard.yml
+++ b/.github/workflows/check_code_standard.yml
@@ -4,6 +4,10 @@
   pull_request:
     types: [opened, synchronize]
 
+permissions:
+  contents: read
+  pull-requests: read
+
 jobs:
   run_check_standard:
     strategy:
EOF
@@ -4,6 +4,10 @@
pull_request:
types: [opened, synchronize]

permissions:
contents: read
pull-requests: read

jobs:
run_check_standard:
strategy:
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +9 to +12
uses: intel/mfd/.github/workflows/check_pr_format.yml@main
with:
REPOSITORY_NAME: ${{ github.event.pull_request.head.repo.full_name }}
BRANCH_NAME: ${{ github.head_ref }} No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

In general, this should be fixed by explicitly setting the GITHUB_TOKEN permissions for the workflow or for the specific job to the minimum required. Since the snippet only shows a job that calls a reusable workflow and we cannot see internals of the called workflow, the safest non-breaking change within this file is to add a conservative permissions block at the job level that grants contents: read only. This preserves the ability to read repository data (which most checks require) while avoiding unnecessary write permissions.

Concretely, in .github/workflows/check_pr_format.yml, under the validate_pr_format: job definition (line 8), add a permissions: section indented to match the job keys, e.g. between the validate_pr_format: line and the uses: line. The block should set contents: read. No imports or other files are involved since this is a YAML workflow file. This change limits the job’s GITHUB_TOKEN to read-only repository contents access unless the called reusable workflow further restricts it.

Suggested changeset 1
.github/workflows/check_pr_format.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/check_pr_format.yml b/.github/workflows/check_pr_format.yml
--- a/.github/workflows/check_pr_format.yml
+++ b/.github/workflows/check_pr_format.yml
@@ -6,6 +6,8 @@
 
 jobs:
   validate_pr_format:
+    permissions:
+      contents: read
     uses: intel/mfd/.github/workflows/check_pr_format.yml@main
     with:
       REPOSITORY_NAME: ${{ github.event.pull_request.head.repo.full_name }}
EOF
@@ -6,6 +6,8 @@

jobs:
validate_pr_format:
permissions:
contents: read
uses: intel/mfd/.github/workflows/check_pr_format.yml@main
with:
REPOSITORY_NAME: ${{ github.event.pull_request.head.repo.full_name }}
Copilot is powered by AI and may make mistakes. Always verify output.

jobs:
dependency_review:
uses: intel/mfd/.github/workflows/dependency_review.yml@main No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

To fix the problem, explicitly define a permissions block that grants only the minimal privileges needed by this workflow. Since this workflow’s only job is to call a reusable workflow for dependency review, and we do not see it performing any writes itself, a conservative and generally safe default is permissions: contents: read at the workflow level. This limits the GITHUB_TOKEN to read-only access to repository contents, which is sufficient for dependency scanning in most setups.

Concretely, in .github/workflows/dependency_review.yml, add a permissions section near the top-level of the workflow, alongside name and on. Insert it after the name line and before the on block (or anywhere at the root level) so that it applies to all jobs, including dependency_review. No new imports or external dependencies are required; we only change the YAML to include:

permissions:
  contents: read

This change preserves existing functionality while constraining the default token permissions used by the reusable workflow.

Suggested changeset 1
.github/workflows/dependency_review.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/dependency_review.yml b/.github/workflows/dependency_review.yml
--- a/.github/workflows/dependency_review.yml
+++ b/.github/workflows/dependency_review.yml
@@ -1,5 +1,8 @@
 name: Dependency Review
 
+permissions:
+  contents: read
+
 on:
   pull_request:
     types: [opened, synchronize]
EOF
@@ -1,5 +1,8 @@
name: Dependency Review

permissions:
contents: read

on:
pull_request:
types: [opened, synchronize]
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +10 to +21
strategy:
fail-fast: false
matrix:
python_version: ['3.10', '3.13']
uses: intel/mfd/.github/workflows/main.yml@main
secrets:
GH_TOKEN: ${{ secrets.GH_TOKEN }}
with:
REPOSITORY_NAME: ${{ github.repository }}
BRANCH_NAME: ${{ github.ref_name }}
PYTHON_VERSION: ${{ matrix.python_version }}
PROJECT_NAME: 'mfd-powermanagement' No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

To fix the problem, explicitly declare the permissions for the GITHUB_TOKEN either at the workflow root or at the job level. Because we only see a single job (build_whl) and must not assume requirements of the reused workflow, the safest generic minimal configuration is to set contents: read for this job. This limits the token to read-only access to repository contents unless more scopes are demonstrably needed.

Concretely, in .github/workflows/main.yml, under jobs: and before strategy: for the build_whl job, add a permissions: block. For example:

jobs:
  build_whl:
    permissions:
      contents: read
    strategy:
      ...

This change keeps the workflow behavior intact (it still calls the same reusable workflow with the same inputs and secrets) while ensuring that the GITHUB_TOKEN used in this job is restricted to read-only repository contents by default. If the reusable workflow truly requires additional permissions (e.g., contents: write or pull-requests: write), they can be added explicitly in that block, but with the information provided, contents: read is the least-privilege starting point and satisfies the CodeQL recommendation.

Suggested changeset 1
.github/workflows/main.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -7,6 +7,8 @@
 
 jobs:
   build_whl:
+    permissions:
+      contents: read
     strategy:
       fail-fast: false
       matrix:
EOF
@@ -7,6 +7,8 @@

jobs:
build_whl:
permissions:
contents: read
strategy:
fail-fast: false
matrix:
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +9 to +20
strategy:
fail-fast: false
matrix:
python_version: ['3.10', '3.13']
uses: intel/mfd/.github/workflows/pull_request.yml@main
secrets:
GH_TOKEN: ${{ secrets.GH_TOKEN }}
with:
REPOSITORY_NAME: ${{ github.event.pull_request.head.repo.full_name }}
BRANCH_NAME: ${{ github.head_ref }}
PYTHON_VERSION: ${{ matrix.python_version }}
PROJECT_NAME: 'mfd-powermanagement' No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

In general, the fix is to add an explicit permissions: block that limits GITHUB_TOKEN to the least privileges required by this workflow. Since this workflow only delegates to another reusable workflow and doesn’t run steps itself, the minimal and safe default is usually contents: read, which allows reading the repository content while avoiding unnecessary write access. If the called workflow needs broader permissions, those should ideally be declared in that reusable workflow; here we only harden the caller.

The best fix with minimal behavior change is to define a root-level permissions: block (so it applies to all jobs) near the top of .github/workflows/pull_request.yml, under the name: key (or at least above jobs:). This will constrain the GITHUB_TOKEN for build_whl unless that job or the called workflow explicitly overrides it. Concretely, in .github/workflows/pull_request.yml, between line 1 (name: Dev Build) and line 3 (on:), insert:

permissions:
  contents: read

No additional imports, methods, or other definitions are needed, since this is pure workflow configuration.

Suggested changeset 1
.github/workflows/pull_request.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
--- a/.github/workflows/pull_request.yml
+++ b/.github/workflows/pull_request.yml
@@ -1,4 +1,6 @@
 name: Dev Build
+permissions:
+  contents: read
 
 on:
   pull_request:
EOF
@@ -1,4 +1,6 @@
name: Dev Build
permissions:
contents: read

on:
pull_request:
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +12 to +21
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
python_version: ['3.10', '3.13']
uses: intel/mfd/.github/workflows/run_tests.yml@main
with:
PYTHON_VERSION: ${{ matrix.python_version }}
RUNS_ON: ${{ matrix.os }}
PROJECT_NAME: 'mfd-powermanagement' No newline at end of file

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {}

Copilot Autofix

AI 3 months ago

To fix the problem, explicitly declare a permissions block to limit the GITHUB_TOKEN to the minimal required scope. Since this workflow only orchestrates a test job and delegates actual work to a reusable workflow, a conservative baseline of contents: read is appropriate unless the tests or reusable workflow are known to need write permissions (which we cannot assume from the snippet). Declaring permissions at the job level ensures that only this job is affected and avoids making assumptions about other workflows.

Concretely, in .github/workflows/run_tests.yml, within the run_tests job definition, add a permissions: mapping between the run_tests: line (line 11) and strategy: (line 12). The new block should set contents: read to follow the least-privilege recommendation. No imports or additional definitions are needed because this is a YAML configuration change only; GitHub Actions natively understands the permissions key.


Suggested changeset 1
.github/workflows/run_tests.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml
--- a/.github/workflows/run_tests.yml
+++ b/.github/workflows/run_tests.yml
@@ -9,6 +9,8 @@
 
 jobs:
   run_tests:
+    permissions:
+      contents: read
     strategy:
       fail-fast: false
       matrix:
EOF
@@ -9,6 +9,8 @@

jobs:
run_tests:
permissions:
contents: read
strategy:
fail-fast: false
matrix:
Copilot is powered by AI and may make mistakes. Always verify output.
@mfd-intel-bot
Copy link
Copy Markdown
Contributor

We don't publish DEVs .whl.
To build .whl, run 'pip install git+https://intel/mfd-powermanagement@snmp'

@mchromin mchromin changed the title Recreate SnmpEngine after each call. fix: Recreate SnmpEngine after each call. Jan 2, 2026
Signed-off-by: Mateusz Chrominski <mateusz.chrominski@intel.com>
@abaczek abaczek merged commit ef3162d into main Jan 8, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants