Skip to content

refactor: do not cascade to TaggedObject#42223

Merged
rusackas merged 1 commit into
apache:masterfrom
hy144328:refactor/sqlalchemy-migration-taggedobject-backref
Jul 20, 2026
Merged

refactor: do not cascade to TaggedObject#42223
rusackas merged 1 commit into
apache:masterfrom
hy144328:refactor/sqlalchemy-migration-taggedobject-backref

Conversation

@hy144328

Copy link
Copy Markdown
Contributor

See #40273 .

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot Bot added the change:backend Requires changing the backend label Jul 19, 2026
@bito-code-review

bito-code-review Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #81bbdd

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 7cdac90..7cdac90
    • pytest.ini
    • superset/tags/models.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - 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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset/tags/models.py
Comment on lines +125 to +130
tag = relationship(
"Tag",
back_populates="objects",
cascade_backrefs=False,
overlaps="tags",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: Add an explicit type annotation for this new ORM relationship attribute so the newly introduced model field is properly typed. [custom_rule]

Severity Level: Minor 🧹

Why it matters? ⭐

The added ORM relationship is new Python code in the PR and it is unannotated. Under the stated rule, new Python code needs proper type hints, so this is a real violation.

Rule source 📖

=== .cursor/rules/dev-standard.mdc === (line 28)

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** superset/tags/models.py
**Line:** 125:130
**Comment:**
	*Custom Rule: Add an explicit type annotation for this new ORM relationship attribute so the newly introduced model field is properly typed.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. In Python, especially when using SQLAlchemy, adding explicit type annotations to model relationships improves code readability and static analysis. You can resolve this by adding the relationship type hint from sqlalchemy.orm.

Would you like me to check the rest of the comments on this PR and implement fixes for them as well?

superset/tags/models.py

from sqlalchemy.orm import relationship
# ...
    tag: relationship = relationship(
        "Tag",
        back_populates="objects",
        cascade_backrefs=False,
        overlaps="tags",
    )

@codecov

codecov Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.37%. Comparing base (42a2aed) to head (f024b81).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #42223      +/-   ##
==========================================
- Coverage   65.19%   64.37%   -0.82%     
==========================================
  Files        2768     2768              
  Lines      156081   156081              
  Branches    35719    35719              
==========================================
- Hits       101754   100481    -1273     
- Misses      52365    53647    +1282     
+ Partials     1962     1953       -9     
Flag Coverage Δ
hive 38.61% <100.00%> (ø)
mysql ?
postgres ?
presto 40.55% <100.00%> (ø)
python 57.68% <100.00%> (-1.65%) ⬇️
sqlite 57.54% <100.00%> (ø)
unit ?

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.

@hy144328
hy144328 force-pushed the refactor/sqlalchemy-migration-taggedobject-backref branch from 7cdac90 to f024b81 Compare July 19, 2026 23:37

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for knocking out another one of these! The codeant annotation nag seems ignorable since that relationship isn't new, just reformatted. Heads up that #42210/#42212/#42213 touch the neighboring pytest.ini lines, so whoever lands second gets a trivial rebase.

@bito-code-review

bito-code-review Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #3ee7a0

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: f024b81..f024b81
    • pytest.ini
    • superset/tags/models.py
  • Files skipped - 0
  • Tools
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - 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 documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@rusackas
rusackas merged commit 36dbff0 into apache:master Jul 20, 2026
66 checks passed
@hy144328
hy144328 deleted the refactor/sqlalchemy-migration-taggedobject-backref branch July 20, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants