You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "kokhlo (via GitHub)" <gi...@apache.org> on 2026/09/18 08:09:46 UTC
[PR] fix(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
kokhlo opened a new pull request, #44424:
URL: https://github.com/apache/superset/pull/44424
### Summary
Fixes the 23 mypy `UUID | None` errors the nightly pre-commit (next) sweep flagged in the four versioning test files (#44423). `UUIDMixin.uuid` (`superset/models/helpers.py:751`) is a nullable `sa.Column`, so mypy infers `UUID | None` for instance reads; the tests passed it (and `BaseDatasource.description`, `str | None`) unnarrowed into strictly-typed callees.
Change is test-only: hoist the read into an annotated local and `assert <local> is not None` before the flagged call, matching the idiom from #44394/#42654. Where a narrowed local already existed (`chart_uuid`, `dataset_uuid`, `slc_uuid`), the assert lands right after its assignment and downstream call sites use the local; `restore_fails_closed_tests.py:375` narrows the `removesuffix` receiver with `(dataset.description or "")` since the target-version suffix is expected on a present description.
### Behavioral analysis
No runtime behavior change intended; every touched entity (chart/slice/dashboard/dataset fixtures) mints its uuid via `default=uuid.uuid4` on insert and is committed before the asserted read, so the asserts are statically-load-bearing but unreachable in practice.
### Test plan
- [x] `mypy --check-untyped-defs` (1.15.0, the pinned pre-commit rev) clean on the 4 files
- [x] `python -c "import ast; ast.parse(...)"` on all 4 files
- [ ] CI green on this PR
### Additional context
Closes #44423. Same defect class as #44384/#44394; surfaced now because the `next` mypy leg only runs on push/schedule (first `--all-files` sweep since #44258/#44251/#44091 merged).
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #44424:
URL: https://github.com/apache/superset/pull/44424
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5780675583
Looks like this just needs the fixes from https://github.com/apache/superset/pull/44513, which are already on master
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "kokhlo (via GitHub)" <gi...@apache.org>.
kokhlo closed pull request #44424: fix(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests
URL: https://github.com/apache/superset/pull/44424
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5770822859
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers an incremental AI Review.
- `/review full` - Manually triggers a full AI Review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5727250827
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44424?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 56.41%. Comparing base ([`bd786e2`](https://app.codecov.io/gh/apache/superset/commit/bd786e2bea2171d517d612babed8ca6a52c97401?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`26f8fb5`](https://app.codecov.io/gh/apache/superset/commit/26f8fb55241485545c9deff4555786e876f696a9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
> :exclamation: There is a different number of reports uploaded between BASE (bd786e2) and HEAD (26f8fb5). Click for more details.
>
> <details><summary>HEAD has 51 uploads less than BASE</summary>
>
>| Flag | BASE (bd786e2) | HEAD (26f8fb5) |
>|------|------|------|
>|hive|9|1|
>|python|44|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44424 +/- ##
===========================================
- Coverage 80.57% 56.41% -24.16%
===========================================
Files 2940 2940
Lines 175316 175316
Branches 40696 40696
===========================================
- Hits 141253 98900 -42353
- Misses 31398 75901 +44503
+ Partials 2665 515 -2150
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44424/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/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.25% <ø> (ø)` | |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/44424/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/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.25% <ø> (-47.63%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/44424/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/44424?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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5769530024
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 4cd73af9c33e4400ff5fb8fbc50853f31a82bf7f |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ab1cb434d0ac3000828d06e |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-44424--superset-docs-preview.netlify.app](https://deploy-preview-44424--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctbCoAgEEDRvcx30pQR0Q5ahjpWQqlM9pBo7_l34XJeiBz2mGAEqECz8mYtPbunviwfLnjnF5HskcSeYxbn6Uh4xRzuMgohG7eQJyqoV7oxupMdoTISEYd2IOwtfD8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-44424--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5727158214
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
Re: [PR] fix(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #44424:
URL: https://github.com/apache/superset/pull/44424#discussion_r4067439243
##########
tests/integration_tests/versioning/restore_fails_closed_tests.py:
##########
@@ -372,12 +373,14 @@ def test_control_without_guard_the_partial_write_happens(self) -> None:
before: str
dataset, column, target_tx, before = self._two_version_dataset()
assert column.description != before
- parent_target_description: str = dataset.description.removesuffix("_v2")
+ dataset_uuid: UUID = dataset.uuid
+ assert dataset_uuid is not None
+ parent_target_description: str = (dataset.description or "").removesuffix("_v2")
Review Comment:
This one falls back to `or ""` instead of the `assert ... is not None` pattern used everywhere else in this diff, so it'd silently produce an empty string instead of failing loud if the fixture ever returned `None` here.
```suggestion
assert dataset.description is not None
parent_target_description: str = dataset.description.removesuffix("_v2")
```
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "kokhlo (via GitHub)" <gi...@apache.org>.
kokhlo commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5769532134
Thanks both for the reviews! Fixed in 4cd73af — replaced the `or ""` fallback with `assert dataset.description is not None` before the use, matching the pattern in the rest of the diff.
(`tests/integration_tests/versioning/restore_fails_closed_tests.py:378`)
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "kokhlo (via GitHub)" <gi...@apache.org>.
kokhlo commented on PR #44424:
URL: https://github.com/apache/superset/pull/44424#issuecomment-5769523795
Thanks for the reviews! Since #44323 was opened 57 seconds before this one with the identical change, deferring to it and closing this PR so the two can't both merge.
One note for that branch: the `or ""` fallback flagged in review at `tests/integration_tests/versioning/restore_fails_closed_tests.py:378` exists in #44323 as well — commit 4cd73af on my branch (`fix/versioning-test-mypy-uuid-narrowing`) shows the fail-loud `assert dataset.description is not None` variant, free to cherry-pick if useful.
--
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(versioning): narrow nullable uuid/description reads flagged by mypy in versioning tests [superset]
Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #44424:
URL: https://github.com/apache/superset/pull/44424#discussion_r4098379213
##########
tests/integration_tests/versioning/restore_fails_closed_tests.py:
##########
@@ -372,12 +373,14 @@ def test_control_without_guard_the_partial_write_happens(self) -> None:
before: str
dataset, column, target_tx, before = self._two_version_dataset()
assert column.description != before
- parent_target_description: str = dataset.description.removesuffix("_v2")
+ dataset_uuid: UUID = dataset.uuid
+ assert dataset_uuid is not None
+ parent_target_description: str = (dataset.description or "").removesuffix("_v2")
Review Comment:
Thanks, Evan — this is addressed in the merged head (`9cab1cf`). The test now asserts `dataset.description is not None` before calling `removesuffix("_v2")`; the `or ""` fallback is gone.
--
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