refactor(BA-5976): generalize filter DTOs and adapter boolean-clause conversion#11509
Draft
refactor(BA-5976): generalize filter DTOs and adapter boolean-clause conversion#11509
Conversation
my_search and project_search inlined filter conversion and dropped DeploymentFilter.AND/OR/NOT. Delegate to _convert_deployment_filter so all three search variants share one correct path.
OR flattened sub-filter conditions into a single OR list, degenerating (A∧B) ∨ (C∧D) into A ∨ B ∨ C ∨ D. NOT had the symmetric problem. Add combine_conditions_and helper. Each OR sub is AND-grouped before OR-combining; each NOT sub is independently negated and pushed to the top-level AND list. Apply to all _convert_*_filter methods in the deployment adapter.
…conversion Centralize the AND/OR/NOT recursive boolean clauses on a shared common.dto.manager.v2.common.BaseFilter and have every v2 entity filter inherit from it, eliminating 30+ near-identical declarations and the matching XxxFilter.model_rebuild() calls. Add convert_and / convert_or / convert_not helpers to BaseFilterAdapter. The OR helper AND-groups each non-empty sub before OR-combining, so adapters that adopt it preserve (A AND B) OR (C AND D) instead of the flattened A OR B OR C OR D shape; the NOT helper negates each sub independently. Migrate all 25 adapters with AND/OR/NOT handling — this also retroactively applies the BA-5975 grouping fix to the 24 adapters where it had not yet been ported. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11506.fix.md -> 11509.fix.md Co-authored-by: octodog <mu001@lablup.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This reverts commit 462ba65.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors v2 filter DTOs and adapter filter conversion to centralize recursive boolean clauses (AND/OR/NOT) and to standardize how adapters convert those clauses into SQLAlchemy boolean expressions (including correct OR branch grouping).
Changes:
- Introduce a shared
BaseFilter(withAND/OR/NOT) and migrate v2 filter DTOs to inherit from it, removing per-filter duplication andmodel_rebuild()calls. - Add
BaseFilterAdapter.convert_and/convert_or/convert_notand migrate adapters to use these helpers (including the OR-branch AND-grouping fix). - Add unit tests for the helper conversions and add/extend component tests to pin deployment adapter filter behavior.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/api/adapters/test_filter_helper.py | Adds unit tests covering convert_and/or/not boolean-shape semantics via compiled SQL output. |
| tests/component/deployment/test_deployment.py | Adds component tests ensuring deployment adapter honors nested AND/OR/NOT (including OR branch grouping). |
| src/ai/backend/manager/repositories/base/utils.py | Adds combine_conditions_and() utility for composing QueryConditions with AND. |
| src/ai/backend/manager/repositories/base/init.py | Re-exports combine_conditions_and from the base repository package. |
| src/ai/backend/manager/api/rest/adapter.py | Adds convert_and/or/not helpers to BaseFilterAdapter using repository composition utilities. |
| src/ai/backend/manager/api/adapters/vfolder/adapter.py | Switches vfolder filter boolean clause conversion to BaseFilterAdapter helpers. |
| src/ai/backend/manager/api/adapters/user/adapter.py | Switches user/keypair filter boolean clause conversion to helpers; updates some converters to instance methods to use helpers. |
| src/ai/backend/manager/api/adapters/session/adapter.py | Switches session/kernel filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/service_catalog/adapter.py | Switches service catalog filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/scheduling_history/adapter.py | Switches scheduling history filters boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/runtime_variant/adapter.py | Switches runtime variant filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/runtime_variant_preset/adapter.py | Switches runtime variant preset filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/resource_usage/adapter.py | Switches resource usage filters boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/resource_preset/adapter.py | Switches resource preset filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/resource_group/adapter.py | Switches resource group filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/rbac/adapter.py | Switches RBAC filters boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/prometheus_query_preset/adapter.py | Switches prometheus query preset filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/project/adapter.py | Switches project filter boolean clause conversion to helpers; updates type filter converter to instance method to use helpers. |
| src/ai/backend/manager/api/adapters/notification/adapter.py | Switches notification filters boolean clause conversion to helpers; updates converters to instance methods to use helpers. |
| src/ai/backend/manager/api/adapters/model_card/adapter.py | Switches model card filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/login_session/adapter.py | Switches login session filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/login_history/adapter.py | Switches login history filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/image/adapter.py | Switches image and image-alias filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/fair_share/adapter.py | Switches fair-share filters boolean clause conversion to helpers; updates converters to instance methods to use helpers. |
| src/ai/backend/manager/api/adapters/domain/adapter.py | Switches domain filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/deployment/adapter.py | Routes my_search/project_search through shared deployment filter conversion and migrates nested boolean handling to helpers. |
| src/ai/backend/manager/api/adapters/deployment_revision_preset/adapter.py | Migrates AND/OR conversion to helpers for preset and allocated-slot filters. |
| src/ai/backend/manager/api/adapters/audit_log/adapter.py | Switches audit log filter boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/artifact/adapter.py | Switches artifact filters boolean clause conversion to helpers. |
| src/ai/backend/manager/api/adapters/agent/adapter.py | Switches agent filter boolean clause conversion to helpers. |
| src/ai/backend/common/dto/manager/v2/common.py | Introduces BaseFilter with recursive AND/OR/NOT fields typed as list[Self]. |
| src/ai/backend/common/dto/manager/v2/vfolder/request.py | Migrates VFolderFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/user/request.py | Migrates UserFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/session/request.py | Migrates SessionFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/service_catalog/request.py | Migrates ServiceCatalogFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild calls. |
| src/ai/backend/common/dto/manager/v2/scheduling_history/request.py | Migrates scheduling history filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/runtime_variant/request.py | Migrates RuntimeVariantFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/runtime_variant_preset/request.py | Migrates RuntimeVariantPresetFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/role_invitation/request.py | Migrates RoleInvitationFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/resource_usage/request.py | Migrates usage bucket filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/resource_slot/request.py | Migrates resource slot-related filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/resource_preset/request.py | Migrates ResourcePresetFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/resource_group/request.py | Migrates ResourceGroupFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/rbac/request.py | Migrates RBAC filters (including nested ones) to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/prometheus_query_preset/request.py | Migrates QueryDefinitionFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/notification/request.py | Migrates notification filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/model_card/request.py | Migrates ModelCardFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/login_session/request.py | Migrates LoginSessionFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/login_history/request.py | Migrates LoginHistoryFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/keypair/request.py | Migrates KeypairFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/kernel/request.py | Migrates KernelFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/image/request.py | Migrates image filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/group/request.py | Migrates ProjectFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/fair_share/request.py | Migrates fair-share filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/domain/request.py | Migrates DomainFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/deployment/request.py | Migrates deployment-related filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/deployment_revision_preset/request.py | Migrates DeploymentRevisionPresetFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/audit_log/request.py | Migrates AuditLogFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/artifact/request.py | Migrates artifact filters to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| src/ai/backend/common/dto/manager/v2/artifact_registry/request.py | Migrates ArtifactFilterInput to inherit from BaseFilter and removes duplicated boolean fields. |
| src/ai/backend/common/dto/manager/v2/agent/request.py | Migrates AgentFilter to inherit from BaseFilter and removes duplicated boolean fields/rebuild. |
| changes/11506.fix.md | Adds changelog entry describing the deployment filter boolean-clause fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
444
to
448
| if filter_.AND: | ||
| for sub in filter_.AND: | ||
| conditions.extend(self._convert_allocated_slot_filter(sub)) | ||
| conditions.extend( | ||
| self.convert_and([self._convert_allocated_slot_filter(sub) for sub in filter_.AND]) | ||
| ) | ||
| if filter_.OR: |
Comment on lines
483
to
487
| if filter_.AND: | ||
| for sub in filter_.AND: | ||
| conditions.extend(self._convert_filter(sub)) | ||
| conditions.extend(self.convert_and([self._convert_filter(sub) for sub in filter_.AND])) | ||
| if filter_.OR: | ||
| or_conds: list[QueryCondition] = [] | ||
| for sub in filter_.OR: | ||
| or_conds.extend(self._convert_filter(sub)) | ||
| if or_conds: | ||
| conditions.append(combine_conditions_or(or_conds)) | ||
| conditions.extend(self.convert_or([self._convert_filter(sub) for sub in filter_.OR])) | ||
| return conditions |
Both _convert_filter (DeploymentRevisionPresetFilter) and _convert_allocated_slot_filter (AllocatedResourceSlotFilter) declared NOT support via the inherited BaseFilter but only walked AND/OR, so NOT sub-filters were silently dropped. Add convert_not() calls to match the other 24 adapters migrated in this PR. Flagged in PR review (#11509). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AND/OR/NOTclauses on a sharedBaseFilterincommon/dto/manager/v2/common.pyand have every v2 entity filter (38 classes across 31 files) inherit from it, eliminating the per-class duplication and the matchingmodel_rebuild()calls.convert_and/convert_or/convert_nottoBaseFilterAdapter. The OR helper AND-groups each non-empty sub-filter before OR-combining ((A AND B) OR (C AND D)rather thanA OR B OR C OR D); the NOT helper negates each sub independently.tests/unit/manager/api/adapters/test_filter_helper.py.Stacked on #11506 (BA-5975); the three BA-5975 commits will fall out of this PR's range once that one merges.
Test plan
pants check --changed-since=origin/mainpants test tests/unit/manager/api/adapters/::OR=[{a,b}, {c,d}]to confirm the BA-5975 grouping fix landed correctlyResolves BA-5976