-
Notifications
You must be signed in to change notification settings - Fork 904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Refactor topology models to include tenant_id in keys. #3923
Open
VladimirFilonov
wants to merge
15
commits into
main
Choose a base branch
from
fix/3898-bug-topology-models-pks-are-tenant-agnostic
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
fix: Refactor topology models to include tenant_id in keys. #3923
VladimirFilonov
wants to merge
15
commits into
main
from
fix/3898-bug-topology-models-pks-are-tenant-agnostic
Conversation
This file contains 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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
8d26ce1
to
9b51c3d
Compare
Matvey-Kuk
reviewed
Mar 10, 2025
keep/api/models/db/migrations/versions/2025-03-07-17-49_aaec81b991bd.py
Outdated
Show resolved
Hide resolved
c4ed400
to
f64a25a
Compare
b49917e
to
3c0fb4e
Compare
Matvey-Kuk
previously approved these changes
Mar 12, 2025
cd2e425
to
c3ba242
Compare
Matvey-Kuk
reviewed
Mar 13, 2025
keep/api/models/db/migrations/versions/2025-03-07-17-49_aaec81b991bd.py
Outdated
Show resolved
Hide resolved
e56dc89
to
9614950
Compare
46ec09d
to
5261448
Compare
Matvey-Kuk
reviewed
Mar 16, 2025
This update introduces "tenant_id" as a required field in all topology models and relationships, ensuring multi-tenancy support. Updated tests, services, and migrations to align with the changes, and added appropriate primary and foreign key constraints for consistency.
The unused `Integer` import was removed to clean up the code and ensure a more maintainable import structure. This change has no impact on functionality but improves code readability.
Temporary tables are renamed to backups to preserve data for potential future use. Comments are added to indicate eventual removal steps if these tables are no longer needed. This change ensures data safety during the interim period.
Corrects the `down_revision` field in the migration file to ensure proper dependency tracking in database migrations. This change resolves potential issues with migration sequencing.
Updated the topology models, migrations, and tests to replace integer-based IDs with UUIDs for better scalability and uniqueness. Introduced an `external_id` field where necessary to preserve compatibility with existing data. Adjusted related logic, schemas, and checks accordingly.
This change updates the test data with the `external_id` field to align with the expected structure. Ensures tests reflect real-world scenarios more accurately.
Updated e2e topology tests for improved node and edge selection logic. Refactored service ID and dependency parameters in type hints from `int` to `UUID | str` for consistency and clarity across various modules.
This commit introduces an empty migration file to merge two divergent database migration branches. It ensures compatibility and aligns the database schema history without making schema changes.
Cleaned up unused imports from the migration file to improve code readability and reduce unnecessary dependencies. This change has no impact on functionality but simplifies the codebase.
Extended wait times and retries in e2e tests to ensure stability and better error handling. Refactored edge creation logic to use dynamic node IDs, improving flexibility and accuracy. Removed primary key from `id` in topology schema to align with database requirements.
Updated migration script to modify parent dependencies and revision ID. Removed the old migration file and replaced it with a new one reflecting correct relationships. No functional changes were introduced.
Eliminated unnecessary imports to clean up the code and improve readability. This change has no functional impact and preserves the script's behavior.
Deleted an obsolete migration file with no operations. Adjusted the `down_revision` in an existing migration to reference the correct dependency, ensuring consistency in migration history.
The `raise` statement within the exception block was unnecessary, as the exception is already handled later in the code. This change improves clarity and avoids redundant code execution.
9104ecc
to
076a115
Compare
Matvey-Kuk
approved these changes
Mar 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scary one, relying on you testing it thoughtfully <3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Architecture
Requires an architecture change/review
size:XL
This PR changes 500-999 lines, ignoring generated files.
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.
This update introduces "tenant_id" as a required field in all topology models and relationships, ensuring multi-tenancy support. Updated tests, services, and migrations to align with the changes, and added appropriate primary and foreign key constraints for consistency.
Closes #3898
📑 Description
✅ Checks
ℹ Additional Information