-
Notifications
You must be signed in to change notification settings - Fork 715
fix: Index stream rbac migration #3959
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
Conversation
WalkthroughThe updates to the codebase primarily focus on enhancing the handling of index operations and migration logic within the Changes
Sequence Diagram(s)sequenceDiagram
participant A as Client
participant B as ofga::init()
participant C as Authz Module
participant D as Resource Manager
A->>B: Call init()
B->>C: Check version for migration
C-->>B: Return migration status
alt Migration required
B->>C: Call get_index_creation_tuples()
C-->>B: Return tuples
end
B->>D: Cleanup resources
D-->>B: Resources cleaned up
B->>C: Set OFGA model
C-->>B: Return success/failure
B-->>A: Return init() status
Note over A,B: Enhanced index operations and error handling
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/common/infra/ofga.rs (4 hunks)
- src/job/files/parquet.rs (2 hunks)
Additional context used
Path-based instructions (2)
src/common/infra/ofga.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
src/job/files/parquet.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Additional comments not posted (8)
src/common/infra/ofga.rs (6)
23-25: Imports look good.The imports for
get_index_creation_tuplesandget_tuple_for_new_indexare necessary for the new functions.
32-33: Usage ofget_tuple_for_new_indexlooks good.The function is correctly used for migrating index streams.
50-58: Logic for checking and migrating index streams looks good.The logic correctly checks if index stream migration is needed based on version comparison.
73-90: Usage ofget_tuple_for_new_indexandget_index_creation_tupleslooks good.The functions are correctly used for processing tuples.
Line range hint
91-133:
Usage ofget_org_creation_tupleslooks good.The function is correctly used for migrating native objects and creating default organizations.
142-151: Usage ofupdate_tupleslooks good.The function is correctly used for updating tuples to openfga.
src/job/files/parquet.rs (2)
72-75: Imports look good.The imports for
AuthzandSchemaRecordsare necessary for the new functionality.
754-759: Function call toset_ownershiplooks good.The function call correctly sets the ownership for the index stream.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/common/infra/ofga.rs (4 hunks)
- src/job/files/parquet.rs (2 hunks)
Additional context used
Path-based instructions (2)
src/common/infra/ofga.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
src/job/files/parquet.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Additional comments not posted (10)
src/common/infra/ofga.rs (8)
32-33: Import new function for index migration.The new function
get_tuple_for_new_indexis imported for use in the migration logic.
35-35: Introduce migration control variable.The variable
need_migrate_index_streamsis introduced to control index stream migration based on version comparison.
50-58: Version comparison logic.The version comparison logic ensures that index stream migration is performed only if the new version is greater than
0.0.4and the existing version is less than0.0.5.
133-136: Enhanced migration logic.The migration logic now includes handling for
get_index_creation_tuplesto ensure proper migration of index streams.
139-151: Improved logging and error handling.The logging and error handling have been improved to provide better visibility into the migration process and handle errors more gracefully.
155-155: Resource cleanup.Ensure that the resource
ris correctly dropped to release any held resources.
23-25: Update imports.The new functions
get_index_creation_tuplesandget_tuple_for_new_indexare now imported. Ensure these functions are correctly implemented and used in the codebase.
73-90: Resource cleanup and migration logic.The logic for resource cleanup and migration has been enhanced. Ensure that the
get_tuple_for_new_indexfunction correctly handles the required tuples for migration.src/job/files/parquet.rs (2)
72-75: AddAuthzimport.The
Authzimport has been added to themetamodule. Ensure that it is correctly used in the codebase.
753-759: Set ownership for index stream.The
set_ownershipfunction is called to set the ownership for the index stream. Ensure that this function correctly handles ownership settings.
c4a205f to
d6e2066
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/common/infra/ofga.rs (4 hunks)
- src/job/files/parquet.rs (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/common/infra/ofga.rs
- src/job/files/parquet.rs
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved index operations and migration handling for better performance and reliability. - Enhanced logging for detailed migration status and tuple information. - **Bug Fixes** - Added error handling to ensure proper setup of the OFGA model, reducing potential failures during initialization. - **Refactor** - Streamlined the `init()` function to handle index stream migrations and resource cleanup more efficiently. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
init()function to handle index stream migrations and resource cleanup more efficiently.