fix: address Dependabot alerts for mlflow, transformers, setuptools - #9032
fix: address Dependabot alerts for mlflow, transformers, setuptools#9032garciadias wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughDependency minimums and compatibility bounds were updated for Transformers, MLflow, and setuptools. Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/handlers/mlflow_handler.py (1)
159-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument and test the new environment-variable contract.
os.environ.setdefaultchanges process-wide MLflow behavior. Add tests verifying that an unset variable defaults to"true"and an existing user value is preserved; also document this side effect in the handler’s Google-style docstring. The existing local-store test does not cover either contract.As per path instructions, modified definitions require appropriate docstrings and unit-test coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@monai/handlers/mlflow_handler.py` around lines 159 - 163, Document the process-wide MLFLOW_ALLOW_FILE_STORE side effect in MLFlowHandler’s Google-style docstring, including that it defaults to "true" only when unset. Extend the MLFlowHandler tests to verify an unset variable becomes "true" and an existing user-provided value remains unchanged, while preserving the current local-store behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@monai/networks/nets/transchex.py`:
- Around line 222-227: Update MultiModal.__init__ to construct the BertConfig
before calling the Transformers base constructor, set
config._attn_implementation to "eager", and pass that config to
super().__init__(config). Add a regression test that instantiates MultiModal
under Transformers 5.5.0 if an appropriate existing test location is available.
---
Nitpick comments:
In `@monai/handlers/mlflow_handler.py`:
- Around line 159-163: Document the process-wide MLFLOW_ALLOW_FILE_STORE side
effect in MLFlowHandler’s Google-style docstring, including that it defaults to
"true" only when unset. Extend the MLFlowHandler tests to verify an unset
variable becomes "true" and an existing user-provided value remains unchanged,
while preserving the current local-store behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b59f4835-ce54-4f75-b753-385bd161b086
📒 Files selected for processing (5)
docs/requirements.txtmonai/handlers/mlflow_handler.pymonai/networks/nets/transchex.pyrequirements-dev.txtrequirements-min.txt
| super().__init__() | ||
| self.config = type("obj", (object,), bert_config) | ||
| self.config = BertConfig(**bert_config) | ||
| # explicitly select the eager attention path: transformers>=4.48 dispatches attention | ||
| # implementations via `config._attn_implementation`, which is otherwise left unset since | ||
| # `bert_config` above does not come from a `from_pretrained` call. | ||
| self.config._attn_implementation = "eager" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
python - <<'PY'
import inspect
from transformers import BertPreTrainedModel
print(inspect.signature(BertPreTrainedModel.__init__))
PYRepository: Project-MONAI/MONAI
Length of output: 282
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate transchex.py =="
fd -a 'transchex.py' . || true
echo "== relevant transchex.py section =="
if [ -f monai/networks/nets/transchex.py ]; then
sed -n '190,245p' monai/networks/nets/transchex.py
fi
echo "== Transformers dependency pins =="
for f in pyproject.toml setup.py setup.cfg requirements.txt requirements.dev.txt environment.yml README.md; do
[ -f "$f" ] && { echo "--- $f"; rg -n "transformers|monai-transformers|torch" "$f" || true; }
done
echo "== repository tests mentioning MultiModal/BertConfig/Transchex =="
rg -n "MultiModal|Transchex|bert_config|BertConfig|transformers" -S --glob '!build/**' --glob '!dist/**' . | head -n 200Repository: Project-MONAI/MONAI
Length of output: 11431
🌐 Web query:
HuggingFace Transformers v5.5.0 PreTrainedModel.__init__ requires config
💡 Result:
In Hugging Face Transformers v5.5.0, the PreTrainedModel.init method strictly requires a config object [1]. When you define a custom model that inherits from PreTrainedModel, your init method must accept a config argument and pass it to super.init(config) [1][2][3]. The library enforces this to ensure that all models have access to shared configuration parameters, such as attention and expert implementation settings, which are processed during the initialization of the base class [1][4]. If you attempt to instantiate a model without a valid config instance, the library will raise a TypeError [1]: Parameter config in {self.class.name}(config) should be an instance of class PreTrainedConfig. To create a model from a pretrained model use model = {self.class.name}.from_pretrained(PRETRAINED_MODEL_NAME) [1] To correctly define and initialize a custom model, follow this pattern [2][3]: class MyCustomModel(PreTrainedModel): config_class = MyCustomConfig def init(self, config): super.init(config) # Initialize your layers here # Proper initialization config = MyCustomConfig model = MyCustomModel(config) If you are using from_pretrained to load a model, the library automatically handles the creation of the config object, which is then passed to your model's init [2][5]. If you encounter issues while defining models in interactive environments like Jupyter notebooks, ensure that your config class is properly defined and registered, as discrepancies in how the config is passed can lead to initialization errors [4].
Citations:
- 1: https://github.com/huggingface/transformers/blob/v5.5.0/src/transformers/modeling_utils.py
- 2: https://huggingface.co/docs/transformers/v5.5.0/custom_models
- 3: https://huggingface.co/docs/transformers/main/custom_models
- 4: Transformers 5.0.0 breaks defining and then initializing custom models in Jupyter notebooks huggingface/transformers#43645
- 5: https://huggingface.co/docs/transformers/v5.5.4/main_classes/model
Pass the config to the Transformers base constructor.
With the required Transformers 5.5.0, PreTrainedModel.__init__ requires config; MultiModal.__init__ assigns self.config after calling super().__init__() with no argument, so the model cannot be constructed. Build the BertConfig before super().__init__() and set _attn_implementation before passing it down.
Proposed fix
- super().__init__()
- self.config = BertConfig(**bert_config)
+ config = BertConfig(**bert_config)
+ config._attn_implementation = "eager"
+ super().__init__(config)
- # explicitly select the eager attention path: transformers>=4.48 dispatches attention
- # implementations via `config._attn_implementation`, which is otherwise left unset since
- # `bert_config` above does not come from a `from_pretrained` call.
- self.config._attn_implementation = "eager"Add a regression test that instantiates MultiModal with Transformers 5.5.0 if supported tests exist.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| super().__init__() | |
| self.config = type("obj", (object,), bert_config) | |
| self.config = BertConfig(**bert_config) | |
| # explicitly select the eager attention path: transformers>=4.48 dispatches attention | |
| # implementations via `config._attn_implementation`, which is otherwise left unset since | |
| # `bert_config` above does not come from a `from_pretrained` call. | |
| self.config._attn_implementation = "eager" | |
| config = BertConfig(**bert_config) | |
| config._attn_implementation = "eager" | |
| super().__init__(config) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@monai/networks/nets/transchex.py` around lines 222 - 227, Update
MultiModal.__init__ to construct the BertConfig before calling the Transformers
base constructor, set config._attn_implementation to "eager", and pass that
config to super().__init__(config). Add a regression test that instantiates
MultiModal under Transformers 5.5.0 if an appropriate existing test location is
available.
Source: Path instructions
…uptools - mlflow: bump floor to >=3.11.1 (drops the <3.0 cap), closing CVEs across the 2.x/early-3.x line. The cap existed for a Python 3.12 packaging bug in mlflow.utils.uv_utils that is no longer present in current releases. mlflow>=3.13 also turns the local file-store warning into a hard error (Project-MONAI#8891); MLFlowHandler now sets MLFLOW_ALLOW_FILE_STORE=true by default since it documents and relies on that local store. Documented the side effect in the class docstring and added tests covering both the unset-defaults-to-true and existing-value-is-preserved cases. - transformers: bump floor to >=5.5.0 (drops the <5.0 cap), closing two HIGH severity CVEs. The cap existed because transformers>=5.x broke Transchex: BertConfig was previously a bare ad-hoc class missing `_attn_implementation`, and BertLayer's forward() return type changed from a tuple to a bare Tensor (the latter was already handled). Fixed both in transchex.py and verified against transformers 4.36-4.40 and 5.5-5.14. The previous <5.0 cap's stated reason (torch.float8_e8m0fnu missing from the nv25.03 Docker image's PyTorch 2.7 build) is unrelated to transchex.py and should be re-verified against the current NGC base image before merging, since it wasn't reproducible against a stock PyPI torch>=2.8.0 install. - setuptools: bump requirements-min.txt floor to >=78.1.1, closing one HIGH severity CVE. Still capped at <=79.0.1 because setuptools>=80 breaks MONAI's own setup.py CLI usage (Project-MONAI#8439); a MEDIUM severity CVE fixed in 83.0.0 remains open until that's resolved. Also drop requirements-dev.txt's separate `setuptools<71` cap, which conflicted with that floor and broke CI dependency installation (mypy, hyena-dep, full-dep): it was added for MetricsReloaded's legacy pkg_resources-based setup.py, but the `monai-support` branch already has that import commented out, and the pinned segment-anything commit never used pkg_resources either, so the cap is no longer needed. Verified via targeted venv testing against the actual pinned versions (transformers==5.5.0, mlflow==3.11.1): tests/networks/nets/test_transchex.py and tests/handlers/test_handler_mlflow.py both pass. CodeRabbit flagged MultiModal.__init__ (transchex.py) for allegedly calling transformers' PreTrainedModel.__init__() without a config, which 5.5.0 requires. That's a false positive: MultiModal subclasses transchex.py's own local `BertPreTrainedModel(nn.Module)` shim, not transformers' class, so HF's config-in-super().__init__() requirement doesn't apply. Confirmed by running the test suite unchanged against transformers 5.5.0 (3 passed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
c81c5f1 to
6317a8c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/handlers/test_handler_mlflow.py (1)
109-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd docstrings to the two new test methods.
test_allow_file_store_env_var_defaults_trueandtest_allow_file_store_env_var_preserves_existinghave no docstrings. State the behavior each test verifies. This matches the existing file convention of undocumented test methods, so treat this as optional polish, not a blocker.As per path instructions, "Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/handlers/test_handler_mlflow.py` around lines 109 - 133, Add Google-style docstrings to the test_allow_file_store_env_var_defaults_true and test_allow_file_store_env_var_preserves_existing test methods. Each docstring should describe the specific behavior being verified: the first should document that MLFlowHandler sets the environment variable to "true" by default when not already set, and the second should document that an existing MLFLOW_ALLOW_FILE_STORE value is preserved and not overwritten by the handler.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/handlers/test_handler_mlflow.py`:
- Around line 109-133: Add Google-style docstrings to the
test_allow_file_store_env_var_defaults_true and
test_allow_file_store_env_var_preserves_existing test methods. Each docstring
should describe the specific behavior being verified: the first should document
that MLFlowHandler sets the environment variable to "true" by default when not
already set, and the second should document that an existing
MLFLOW_ALLOW_FILE_STORE value is preserved and not overwritten by the handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b4b3757d-3dad-4456-9c66-8f1d654db887
📒 Files selected for processing (6)
docs/requirements.txtmonai/handlers/mlflow_handler.pymonai/networks/nets/transchex.pyrequirements-dev.txtrequirements-min.txttests/handlers/test_handler_mlflow.py
🚧 Files skipped from review as they are similar to previous changes (4)
- monai/handlers/mlflow_handler.py
- requirements-min.txt
- monai/networks/nets/transchex.py
- docs/requirements.txt
1 participant
Summary
Addresses the open Dependabot alerts for
mlflow,transformers, andsetuptools(28 open alerts as of this writing, all in dev/docs/test dependency files — no runtime imports inmonai/itself are affected beyond the two files touched below).Before bumping any floor, I checked out why each cap existed (
#8912,#8891,#8441/#8439) and reproduced the actual blocker in an isolated venv rather than assuming a version-number bump was safe. That changed the plan twice:transformers<5.0cap turned out to be caused by two independent issues, not one.BertLayer.forward()'s return type change (tuple →Tensor) was already fixed here in Update base image to 25.12 #8738's_get_hidden_states. The second —MultiModal.configbeing an ad-hoctype("obj", (object,), bert_config)instead of a realBertConfig, which crashes onconfig._attn_implementationin transformers>=4.48's attention dispatch — was not. Fixed in this PR.mlflow<3.0/<3.13caps' root causes (a Python 3.12 packaging bug, and MLFlow 3.13 Raises Exception When Using Filesystem Backend #8891's file-store hard-error) are handled differently: the packaging bug appears fixed upstream already; MLFlow 3.13 Raises Exception When Using Filesystem Backend #8891 is still open, so I fixed it directly inMLFlowHandler.Fixed
transformers4.53.0→5.5.0inrequirements-dev.txt/docs/requirements.txt. Required fixingmonai/networks/nets/transchex.py:MultiModal.__init__builtself.configas a baretype("obj", (object,), bert_config), which has no_attn_implementationattribute — transformers>=4.48's attention-interface dispatch (ALL_ATTENTION_FUNCTIONS.get_interface(self.config._attn_implementation, ...)) raisesAttributeErroron it. Switched to a realtransformers.BertConfig(**bert_config)with_attn_implementationexplicitly set to"eager". Verifiedtests/networks/nets/test_transchex.pypasses against both transformers 4.36–4.40 (Python 3.10 venv) and transformers 5.5–5.14 (Python 3.14 venv).mlflow2.12.2→3.11.1(drops the<3.0/<3.13caps) inrequirements-dev.txt/docs/requirements.txt. The original<3.0cap (#8912) was for a Python 3.12 packaging bug inmlflow.utils.uv_utils(from .. import zippat invalid scope) — I couldn't reproduce this against the currently installed mlflow 3.14.0 and found no such import in its source, so it appears already fixed upstream (I don't have a Python 3.12 interpreter available to confirm directly under that exact version — worth a quick sanity check in CI). Separately, mlflow>=3.13 turns the local file-store warning into a hardMlflowExceptionunlessMLFLOW_ALLOW_FILE_STORE=true(#8891, still open) —MLFlowHandler.__init__now sets this by default, since it documents and defaults to exactly that localmlrunsstore. Verifiedtests/handlers/test_handler_mlflow.py(7/8 tests; the 8th needs network access to download test data, unrelated to this change) against mlflow 3.14.0.setuptools50.3.0/70.2.0→78.1.1inrequirements-min.txt, closing this for the min-dependency test environment. Does not close it for the full dev/test environment — see below.Not fixable — no upstream patch exists yet
mlflow/ajax-api/3.0/jobs/*unauthenticated access — no fixed version listed by the advisory.mlflowmlflowMONAI only uses
mlflowas a tracking client (monai/handlers/mlflow_handler.py) — it doesn't run the vulnerable tracking-server / AI-Gateway endpoints these three describe, so real-world exposure through MONAI itself is low even while they stay open upstream.Not fixable in this PR — blocked by other constraints
setuptools>=83.0.0. Blocked byProject-MONAI/MONAI#8439:setuptools>=80removed the legacysetup.pyCLI invocation (fetch_build_eggs) that MONAI's own C++/CUDA extension build still uses, sorequirements-min.txtis capped at<=79.0.1(see#8441). Needs MONAI's ownsetup.pymodernized first.requirements-dev.txt)setuptoolsrequirements-dev.txthas its ownsetuptools<71line, needed becauseProject-MONAI/MetricsReloaded'ssetup.pystill usespkg_resources, removed insetuptools>=71. This requires a fix in the MetricsReloaded repo itself, which this PR can't touch. I'd suggest filing that as a separate issue there.Please re-verify before merging
I don't have access to the
nvcr.io/nvidia/pytorchNGC base image this repo builds tutorials against. The originaltransformers<5.0cap comment (#8912) citedtorch.float8_e8m0fnumissing from the NGC image's bundled PyTorch 2.7 build. I could not reproduce this against a stock PyPItorch>=2.8.0install (confirmedfloat8_e8m0fnuis present from torch 2.8.0 onward), which is whatrequirements.txtactually declares — but the NGC image may bundle something different from PyPI. Please re-run the Docker/tutorial CI (the same one that caught this in #8912) before merging.Test plan
tests/networks/nets/test_transchex.pypasses against transformers 4.36–4.40 and transformers 5.5–5.14 (isolated venvs, CPU torch)tests/handlers/test_handler_mlflow.pypasses (7/8; 8th needs network access) against mlflow 3.14.0transformers>=5.5.0bump doesn't hit the NGC-image-specifictorch.float8_e8m0fnuissue from fix: update Dockerfile and requirements-dev.txt for MONAI 1.6 tutorial compatibility #8912uv_utilsPython 3.12 packaging bug is actually resolved in CI (I verified by reading source, not by reproducing under Python 3.12 directly)🤖 Generated with Claude Code