[tannnmayy]

Fixes #40854

import resource and the ModelManager import shared one try block in
apache_beam/ml/inference/base.py. When ModelManager's dependencies (torch)
are missing, the except branch also set resource = None, so on Linux/macOS
RunInference reported 0 for model memory (model_byte_size) and logged
"Resource module is not available..." twice per model load.

This splits the block so each import falls back independently, and adds a
RunInference test that patches ModelManager to None and checks that the
"Resource module is not available" warning isn't logged.

Testing:

• Without torch, the new test fails without the fix and passes with it
  (Linux, Python 3.14).
• base_test.py passes on Linux and Windows (the new test is skipped on
  Windows, where resource doesn't exist).
• ruff, yapf and pyrefly report no issues on the changed files.

The fix and the regression test were written by me and I independently reviewed everything and ran the checks multiple times,
I did use my ai to help me navigate beam and to learn more about it, since this is my first time contributing to beam.

[github-actions]

📋 Review Summary

This PR cleanly resolves an issue where a failed import of ModelManager (e.g., when optional dependencies like torch are not installed) inadvertently set the resource module reference to None, breaking model_byte_size metric collection on POSIX platforms. Splitting the try...except ImportError blocks and verifying the behavior in an isolated subprocess test is the right approach.

🔍 General Feedback

• The separation of import resource and from apache_beam.ml.inference.model_manager import ModelManager is clean and preserves the fallback behavior for both modules independently.
• The regression test in base_test.py accurately simulates the missing dependency scenario using a fresh interpreter subprocess and properly skips on Windows.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[tannnmayy]

/gemini review

[github-actions]

📋 Review Summary

This PR fixes a bug in apache_beam.ml.inference.base where a shared try...except ImportError block caused resource to be set to None whenever ModelManager failed to import (e.g., when optional dependencies like torch are absent). Separating the two try...except blocks and adding an isolated subprocess unit test cleanly resolves the issue.

🔍 General Feedback

• The separated import handling is clean and preserves platform fallback behavior for both resource (on Windows) and ModelManager (when torch/scipy are not installed).
• The regression test using subprocess.run and sys.modules['apache_beam.ml.inference.model_manager'] = None is a great way to test module-level import state without polluting the test runner's sys.modules.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[github-actions]

Assigning reviewers:

R: @shunping for label python.
Backup reviewer: jrmccluskey (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.81%. Comparing base (0f1ac43) to head (d03d8a2).
⚠️ Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
sdks/python/apache_beam/ml/inference/base.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #40855       +/-   ##
===========================================
+ Coverage        0   79.81%   +79.81%     
===========================================
  Files           0      494      +494     
  Lines           0    83575    +83575     
===========================================
+ Hits            0    66709    +66709     
- Misses          0    16866    +16866     

Flag	Coverage Δ
python	79.81% <33.33%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[shunping]

Welcome to Beam and thanks for your contribution! @tannnmayy

I think the test is trivial and I would just remove that.

Instead, have you considered adding a unit test to RunInference to see if we will get warnings if ModelMananger is not importable but resource is?

If it is doable and the test is simple (without using subprocess), I would rather have that.

[tannnmayy]

Thanks for the welcome! Makes sense. I removed the subprocess test and added test_no_resource_warning_when_model_manager_unavailable to RunInferenceBaseTest. It patches ModelManager to None, runs RunInference with FakeModelHandler, and checks that the "Resource module is not available" warning isn't logged. It's skipped on Windows, where resource doesn't exist.

Without torch, as in the default test environment, it fails on the old code and passes with the fix. With torch installed it passes either way, since the original bug happened at import time.

I also merged master and moved the CHANGES.md entry to 2.79.0, since 2.78.0 has been cut.

[shunping]

LGTM, thanks! I will merge once all of the tests are green.