You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/07/08 15:32:07 UTC
[PR] fix(mcp): patch list_metrics DAOs via module object to avoid Python 3.10 mock resolution bug [superset]
rusackas opened a new pull request, #41881:
URL: https://github.com/apache/superset/pull/41881
### SUMMARY
`Python-Unit` has been intermittently failing on master's `unit-tests (previous)` job (Python 3.10) since ~2026-07-07T16:51, with:
```
AttributeError: <function list_metrics at 0x...> does not have the attribute 'DatasetDAO'
```
Root cause: `superset/mcp_service/semantic_layer/tool/__init__.py` re-exports the `list_metrics` function under the same name as its defining submodule:
```python
from superset.mcp_service.semantic_layer.tool.list_metrics import (
list_metrics,
)
```
This overwrites the `tool.list_metrics` package attribute (originally set to the submodule by Python's import machinery) with the function object. `tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py` patches `DatasetDAO`/`SemanticViewDAO`/`db` via the dotted string `"superset.mcp_service.semantic_layer.tool.list_metrics.DatasetDAO"`. On Python 3.10, `unittest.mock`'s target resolution (`_importer`) walks the dotted path via plain `getattr`, so it resolves `tool.list_metrics` to the shadowed function instead of the submodule, then fails looking up `DatasetDAO` on it. This was fixed in a later CPython version (mock's resolution now prefers importing the submodule directly), so the bug only reproduces on 3.10 — hence the flip-flopping pass/fail pattern across the Python version matrix.
Confirmed with a minimal repro locally: fails identically on Python 3.10.14, passes on 3.13.0a5.
The fix switches the affected patches to `patch.object(list_metrics_module, "X")` against the module reference this file already imports at the top (`importlib.import_module(...)`) — the same pattern this file already uses for `user_can_view_data_model_metadata`. This sidesteps the ambiguous string resolution entirely rather than depending on Python-version-specific mock internals.
Checked sibling test files in the same package (`test_get_table.py`, `test_get_compatible_metrics.py`, `test_get_compatible_dimensions.py`) for the same footgun — they patch DAOs at their defining module (e.g. `superset.daos.dataset.DatasetDAO`) rather than through the re-exporting package, so they're unaffected.
### TESTING INSTRUCTIONS
On Python 3.10:
```
pytest tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py -v
```
Previously failed 6 tests with the `AttributeError` above; should now pass. `pre-commit run --files tests/unit_tests/mcp_service/semantic_layer/tool/test_list_metrics.py` passes clean (ruff, mypy).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): patch list_metrics DAOs via module object to avoid Python 3.10 mock resolution bug [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41881:
URL: https://github.com/apache/superset/pull/41881#issuecomment-4916680482
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41881?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.87%. Comparing base ([`34d4966`](https://app.codecov.io/gh/apache/superset/commit/34d4966f8b10ca7b6a62ccc12866c95051d82d20?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`170c8db`](https://app.codecov.io/gh/apache/superset/commit/170c8db646fdcc9c2d9d0f271edc4bef5f1fbe72?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41881 +/- ##
==========================================
- Coverage 64.08% 63.87% -0.22%
==========================================
Files 2702 2702
Lines 150222 150222
Branches 34594 34594
==========================================
- Hits 96276 95960 -316
- Misses 52157 52485 +328
+ Partials 1789 1777 -12
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.06% <ø> (ø)` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.57% <ø> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.37% <ø> (-0.44%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/41881/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.22% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41881?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): patch list_metrics DAOs via module object to avoid Python 3.10 mock resolution bug [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41881:
URL: https://github.com/apache/superset/pull/41881
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(mcp): patch list_metrics DAOs via module object to avoid Python 3.10 mock resolution bug [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41881:
URL: https://github.com/apache/superset/pull/41881#issuecomment-4916465411
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org