feat: qol improvements & fixes #166
Conversation
📝 WalkthroughWalkthroughMultiple new JSON metadata files were added for prerelease versions of various Apibara packages, including plugins for Drizzle, Mongo, and SQLite, as well as the core Apibara package. Key changes include enhanced ID column mapping support, revised dependency versions, and refined transaction and cleanup flows. Additionally, the CLI commands were updated to include extra parameters and a new argument validation function, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as CLI Command
participant V as checkForUnknownArgs
participant E as Execution Logic
U->>C: Run command with args & cmd
C->>V: Validate arguments (args, cmd)
V-->>C: Return validation (or exit on error)
C->>E: Proceed with command operations
sequenceDiagram
participant D as DrizzleStorage
participant M as getIdColumnForTable
participant V as Validation
D->>M: Request id column for table from idColumnMap
M-->>D: Return specific id column (or fallback)
D->>V: Validate id column against schema
V-->>D: Confirm validity or throw error
D->>D: Execute storage operations using mapped id columns
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Definitions (1)packages/plugin-drizzle/src/index.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (12)
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli/src/cli/commands/init.ts (1)
23-28: Replace placeholder description for create-indexer flag.The argument has been renamed from
noIndexertocreate-indexerwhich is more intuitive (positive wording instead of negative), but the description is set to "TODO" and should be replaced with a proper description explaining what this flag does."create-indexer": { type: "boolean", name: "create-indexer", default: true, - description: "TODO", + description: "Create an indexer as part of the project initialization | default: `true`", },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
change/@apibara-plugin-drizzle-fb22b1bc-f1ab-4299-a476-2ac25460956d.json(1 hunks)change/@apibara-plugin-mongo-c9018c01-2b4f-4a5a-a7a7-e13eb567d66f.json(1 hunks)change/@apibara-plugin-sqlite-22633833-27a0-4ae1-be3b-824b3514f5e2.json(1 hunks)change/apibara-34f7f808-dcf6-45ec-bbe4-5e74b0193361.json(1 hunks)examples/cli-drizzle/indexers/2-starknet.indexer.ts(1 hunks)examples/cli-drizzle/package.json(1 hunks)packages/cli/src/cli/commands/add.ts(2 hunks)packages/cli/src/cli/commands/build.ts(2 hunks)packages/cli/src/cli/commands/dev.ts(2 hunks)packages/cli/src/cli/commands/init.ts(2 hunks)packages/cli/src/cli/commands/prepare.ts(2 hunks)packages/cli/src/cli/commands/start.ts(2 hunks)packages/cli/src/cli/common.ts(1 hunks)packages/cli/src/create/constants.ts(1 hunks)packages/plugin-drizzle/src/index.ts(8 hunks)packages/plugin-drizzle/src/storage.ts(8 hunks)packages/plugin-drizzle/src/utils.ts(1 hunks)packages/plugin-mongo/src/index.ts(1 hunks)packages/plugin-sqlite/src/index.ts(9 hunks)packages/plugin-sqlite/src/kv.ts(2 hunks)packages/plugin-sqlite/src/persistence.ts(2 hunks)packages/plugin-sqlite/tests/kv.test.ts(4 hunks)
🧰 Additional context used
🧬 Code Definitions (8)
packages/cli/src/cli/commands/add.ts (1)
packages/cli/src/cli/common.ts (1) (1)
checkForUnknownArgs(16-29)
packages/cli/src/cli/commands/build.ts (1)
packages/cli/src/cli/common.ts (1) (1)
checkForUnknownArgs(16-29)
packages/plugin-sqlite/src/persistence.ts (3)
packages/plugin-mongo/src/persistence.ts (1) (1)
resetPersistence(161-183)packages/plugin-drizzle/src/persistence.ts (1) (1)
resetPersistence(322-341)packages/plugin-sqlite/src/utils.ts (1) (1)
assertInTransaction(27-31)
packages/cli/src/cli/commands/start.ts (1)
packages/cli/src/cli/common.ts (1) (1)
checkForUnknownArgs(16-29)
packages/cli/src/cli/commands/prepare.ts (1)
packages/cli/src/cli/common.ts (1) (1)
checkForUnknownArgs(16-29)
packages/cli/src/cli/commands/dev.ts (1)
packages/cli/src/cli/common.ts (1) (1)
checkForUnknownArgs(16-29)
packages/plugin-drizzle/src/storage.ts (2)
packages/plugin-drizzle/src/utils.ts (2) (2)
IdColumnMap(52-57)getIdColumnForTable(59-69)packages/plugin-drizzle/src/constants.ts (1) (1)
SCHEMA_NAME(3-3)
packages/plugin-sqlite/src/kv.ts (2)
packages/plugin-sqlite/src/utils.ts (1) (1)
assertInTransaction(27-31)packages/protocol/src/common.ts (2) (2)
Cursor(44-44)Cursor(45-45)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (60)
change/@apibara-plugin-mongo-c9018c01-2b4f-4a5a-a7a7-e13eb567d66f.json (1)
1-7: LGTM! Change metadata file correctly documents MongoDB plugin changes.The metadata file properly documents the prerelease change for the MongoDB plugin, specifically moving the cleanup part after initialization. This aligns with the PR objective to fix cleanup query errors that occur before initialization when using the
--always-reindexflag.change/apibara-34f7f808-dcf6-45ec-bbe4-5e74b0193361.json (1)
1-7: LGTM! Change metadata covers multiple CLI improvements.This metadata file correctly documents several core improvements mentioned in the PR objectives:
- Fixing dev reload issues
- Changing flag names in CLI
- Adding arguments validation
- Updating drizzle-orm version
The prerelease designation with patch change type is appropriate for these types of improvements.
packages/cli/src/create/constants.ts (1)
82-82: LGTM! Drizzle-orm version updated appropriately.The drizzle-orm dependency has been updated from ^0.37.0 to ^0.40.1. This version bump matches one of the stated objectives in the PR and should provide the latest features and bug fixes from the drizzle-orm library.
examples/cli-drizzle/package.json (1)
34-34: LGTM! Version update is consistent with template definition.The drizzle-orm version update to ^0.40.1 in the example project matches the same update made in the template constants file. This consistency ensures that generated projects will use the same dependency version as the examples.
packages/cli/src/cli/commands/add.ts (2)
3-3: Added import for argument validationThis import brings in the validation function that will check for unknown CLI arguments.
39-41: Implemented unknown argument validationAdded proper validation for unknown CLI arguments, which fulfills the PR objective about handling unknown arguments. The implementation checks arguments against the command definition, showing an error and usage information when unknown arguments are provided.
packages/cli/src/cli/commands/build.ts (2)
5-5: Import for argument validation addedThe new import
checkForUnknownArgsfrom "../common" enables validation of command-line arguments, which aligns with the PR objective of handling unknown arguments in the CLI.
15-17: Command validation implementationThe run method now accepts the command context and validates arguments before execution, providing better error handling and user feedback when invalid arguments are passed. This addresses the PR objective of validating unknown CLI arguments.
packages/cli/src/cli/commands/prepare.ts (2)
5-5: Import for argument validation addedThe
checkForUnknownArgsimport enables the same validation pattern seen in other commands, maintaining consistency across the CLI interface.
15-17: Command validation implementation in prepare commandAdded argument validation before main command execution, consistent with the pattern implemented in other commands. This ensures that invalid arguments trigger helpful error messages rather than unexpected failures.
packages/cli/src/cli/commands/start.ts (3)
7-7: Import for argument validation addedAdding the
checkForUnknownArgsimport to enable consistent argument validation across commands.
26-26: Fix for process exiting issue and argument validationTwo important changes here:
- The
cmdparameter addition enables argument validation- The validation is performed before executing the command logic
This implementation, combined with the removal of the explicit
process.exit()call in the child process close handler (based on the PR objectives), addresses the main process exiting issue during development reloads mentioned in the PR description.Also applies to: 30-31
65-72: Fixed process exit handlingThe removal of the explicit
process.exit()call (previously after line 71) resolves the issue with the main process exiting during development reloads, as mentioned in the PR objectives. The process will now continue running even after child processes exit, which is especially important during development with hot reloading.packages/plugin-sqlite/tests/kv.test.ts (1)
55-55: Added indexerId to KV Store recordsThe test cases have been updated to include the
idfield with value "indexer_testing_default" in all key-value store records. This change validates that theindexerIdrelation has been correctly implemented in the KV Store as mentioned in the PR objectives.Also applies to: 62-62, 69-69, 76-76, 83-83, 90-90, 276-276, 283-283, 290-290, 461-461, 468-468, 475-475, 482-482, 566-566, 573-573
packages/cli/src/cli/commands/init.ts (2)
3-3: Good addition of validation for unknown arguments.Adding the import for
checkForUnknownArgsis a positive improvement that allows the command to validate and handle unknown arguments, which aligns with the PR objectives for better CLI argument handling.
30-38: Good update to method signature and logic inversion.The changes correctly:
- Update the method signature to include the
cmdparameter- Add validation for unknown arguments
- Update the destructuring to use the renamed parameter
- Invert the logic to match the renamed parameter (from
noIndexerto!createIndexer)This implementation aligns with the PR objectives to improve CLI argument handling.
packages/plugin-drizzle/src/utils.ts (2)
52-57: Good implementation of the IdColumnMap interface.The
IdColumnMapinterface provides a flexible way to map table names to their respective ID column names, with the wildcard property ("*") serving as a fallback mechanism. This is a well-designed type that supports the PR objective of adding "Support for a specificidColumnmap for tables... including wildcard support."
59-69: Clean implementation of the getIdColumnForTable function.The function follows a simple and effective pattern:
- Check for a specific mapping for the provided table name
- Fall back to the wildcard mapping if no specific mapping exists
This correctly implements the feature mentioned in the PR objectives about specific
idColumnmap support with wildcard capability.packages/plugin-sqlite/src/persistence.ts (2)
103-111: Robust implementation of resetPersistence function.The
resetPersistencefunction correctly:
- Accepts a database and indexer ID as parameters
- Asserts that the database is in a transaction (important for data integrity)
- Uses prepared statements to delete checkpoints and filters for the specific indexer
This implementation matches similar functions in other plugins (Mongo, Drizzle) and supports the PR objective of fixing "cleanup query errors that occur before initialization when the
--always-reindexflag is activated."
181-186: Appropriate SQL statements for persistence reset.The SQL statements are correctly defined to delete records from the checkpoints and filters tables based on the indexer ID. These statements are essential for the
resetPersistencefunction to work properly.packages/cli/src/cli/commands/dev.ts (5)
8-8: Comprehensive import update.The updated import now includes both
checkForUnknownArgsandcommonArgs, which facilitates argument validation and reuse of common argument definitions, supporting the PR objectives for improved CLI argument handling.
29-34: Good renaming of always-reindex flag for consistency.The argument has been renamed from
alwaysReindextoalways-reindexfor consistency with other hyphenated argument names, and the description has been slightly refined for clarity.
36-38: Proper method signature update and validation addition.The method signature now includes additional parameters (
data,cmd, andrawArgs) that provide more context and enable argument validation. The added validation withcheckForUnknownArgschecks for unknown arguments and displays a help message, fulfilling the PR objective for handling unknown CLI arguments.
41-43: Correctly updated conditional check.The condition has been updated to use
args["always-reindex"]instead ofargs.alwaysReindex, reflecting the argument renaming.
103-131: Fixed main process exiting issue during development reloads.The removal of
process.exit(code ?? 0)from thedev:reloadhook prevents the main process from exiting unexpectedly during development reloads, addressing one of the key objectives mentioned in the PR description.packages/plugin-mongo/src/index.ts (1)
73-92: Well-structured transaction flow for dealing with initialization and cleanupThe code now properly handles initialization and cleanup inside a single transaction. This change ensures that everything happens in the correct order, preventing the issue where cleanup queries could fail before initialization when
--always-reindexis activated.The transaction now:
- Initializes the persistent state first (if enabled)
- Performs cleanup operations after initialization
- Ensures all related operations are atomic
packages/cli/src/cli/common.ts (2)
1-7: Good addition of required imports for the new validation functionalityThe imports have been updated to include the necessary types and functions from
cittyandconsolalibraries that will be used for argument validation.
16-29: Excellent implementation of CLI argument validationThis new function adds robust validation for unknown CLI arguments. It will:
- Compare provided arguments against defined arguments
- Show a meaningful error message for unknown arguments
- Display the usage information to guide the user
- Exit the process to prevent incorrect operation
This implementation aligns with the PR objective to "handle unknown arguments passed in the CLI" and provides a better developer experience.
packages/plugin-sqlite/src/index.ts (3)
2-2: Proper import updates for the new functionalityThe imports have been updated to include:
useLogger- for logging cleanup operationscleanupKV- for cleaning up the key-value storeresetPersistence- for resetting the persistence stateThese additions support the new cleanup and reindexing functionality.
Also applies to: 10-10, 21-21
82-82: Well-implemented cleanup logic for reindexingThe implementation now properly handles the
--always-reindexflag by:
- Correctly determining if reindexing is needed
- Using a logger to inform users about the cleanup process
- Cleaning up the key-value store based on the indexerId
- Resetting the persistence state
- Using a flag to ensure cleanup is only applied once
- Providing clear success messages
This change resolves the issue with cleanup query errors that could occur before initialization.
Also applies to: 88-89, 94-121
171-172: Proper indexerId integration with KV Store operationsThe indexerId parameter has been added to:
invalidateKVcallsfinalizeKVcalls- The KeyValueStore constructor
This change ensures proper data isolation between different indexers sharing the same database, allowing for more robust multi-indexer setups.
Also applies to: 208-209, 226-227, 264-265
packages/plugin-drizzle/src/storage.ts (3)
19-23: Updated imports to support idColumn mappingThe import section has been properly updated to include
IdColumnMaptype andgetIdColumnForTablefunction, which are essential for the new table-specific ID column functionality.
123-151: Enhanced flexibility with table-specific ID column mappingThe
registerTriggersfunction now accepts anidColumnMapinstead of a singleidColumnstring, enabling support for different ID columns per table. The implementation:
- Uses
getIdColumnForTableto find the correct ID column for each table- Properly applies the table-specific ID column in the trigger SQL statement
- Maintains compatibility with the previous behavior through the wildcard mapping
This change aligns with the PR objective to add "support for a specific
idColumnmap for tables" with wildcard support.
186-220: Consistent application of table-specific ID columns in invalidation logicThe
invalidatefunction has been updated to use theidColumnMapparameter consistently:
- The function signature correctly uses
idColumnMap: IdColumnMap- Table-specific ID columns are determined for each operation using
getIdColumnForTable- The determined column is consistently used in SQL queries for DELETE statements
- The update logic properly filters non-ID keys based on the table-specific ID column
This implementation ensures that invalidation operations work correctly with the flexible ID column mapping system.
Also applies to: 284-286, 295-297
packages/plugin-drizzle/src/index.ts (12)
35-41: Imports appear correct and comprehensive.
ImportingDrizzleStorageError,IdColumnMap,getIdColumnForTable,sleep, andwithTransactionfrom"./utils"is a good organizational choice, centralizing utility logic and custom errors.
45-45: ExportingIdColumnMapis beneficial for external use.
This export helps consumers define or validate custom ID column mappings in their own code.
100-125: Doc comment enhancements provide clear usage instructions.
Allowing a single string or a mapping object foridColumnaccommodates more complex schema setups. The detailed documentation and code examples are very helpful.
154-155: Assigning_schemaandidColumnfor internal use is straightforward.
No issues found with this parameter passing.
163-167: Logical fallback when constructingidColumnMap.
Using"*"as the default wildcard is consistent with documentation, and layering user-supplied mappings on top ensures each table’s ID column is found.
170-170: Fetching table names from the schema.
CollectingdbNamevalues is a common approach. The surrounding try-catch ensures robust error handling.
178-192: Validation of the ID column’s existence in each table.
ThrowingDrizzleStorageErrorwhen columns do not match prevents runtime inconsistencies. This is a sound approach that ensures correctness.
211-211: IntroducingcleanupAppliedflag improves reindexing logic.
This boolean gate helps avoid repeated cleanup for subsequent hooks, mitigating accidental data loss.
226-241: Cleanup flow on reindexing appears well-structured.
Deleting existing table data and resetting persistence once ensures a consistent fresh start. The warnings and logs are clear.
295-296: UtilizinginvalidatewithidColumnMap.
Passing an explicit mapping ensures each table’s primary key is accurately invalidated.
346-347: Consistent usage of theinvalidatefunction.
Mirrors the pattern above to ensure correct rollback of pending data.
385-385: Registering triggers withidColumnMapfosters table-specific tracking.
Providing the map here ensures that triggers are correctly keyed to each table’s identifier.packages/plugin-sqlite/src/kv.ts (11)
22-22: StoringindexerIdstrengthens scoping of key-value operations.
This allows multiple indexers to share the same table without overlapping data.
28-30: Retrieving rows now scoped byindexerId.
Ensuring theindexerIdis included in the selection query prevents accidental data mixing.
37-38: Updating theto_blockcolumn conditionally byindexerId.
This helps ensure an indexer only affects its own rows during block updates.
41-43: Insert statement expanded to handleindexerId.
Insertingidallows for clearly partitioned data, and addingindexerIdhere is consistent with the new scoping mechanism.Also applies to: 48-48
54-55: Delete operation (del) also scoped byindexerId.
This extends the same data isolation principles to record deletion.
59-59:finalizeKVnarrowing deletes based onindexerId.
The updated function signature and query ensure finalization only applies to the relevant indexer’s records.Also applies to: 62-64
68-68:invalidateKVupdated to requireindexerId.
The function signature improvement is consistent with the new scoping approach inKeyValueStore.
84-88:cleanupKVintroduced to remove a single indexer’s data.
Good functionality for fully clearing stored states, especially for reindex scenarios.
95-95:idproperty inKeyValueRow.
Integratingid(an indexer-specific identifier) aligns with the new primary key arrangement.
105-106: Amending schema foridcolumn and primary key.
This ensures each record is now uniquely identified by(from_block, k, id), supporting multi-indexer data separation.
111-111: SQL statements updated to incorporateidin all operations.
All queries now condition onidfor retrieval, updates, and cleanup, providing proper isolation across indexers.Also applies to: 115-115, 117-118, 122-122, 125-125, 128-128, 132-132, 133-136
change/@apibara-plugin-sqlite-22633833-27a0-4ae1-be3b-824b3514f5e2.json (1)
1-7: Metadata for SQLite plugin changes looks goodThe new change file correctly documents the nature of the changes to the SQLite plugin, specifically the addition of cleanup logic and indexerId relation to KV Store. This aligns with the PR objectives mentioned.
change/@apibara-plugin-drizzle-fb22b1bc-f1ab-4299-a476-2ac25460956d.json (1)
1-7: Metadata for Drizzle plugin changes is properly documentedThe change file accurately reflects the additions of idColumn map support with validation and the fix for cleanup errors, which matches the PR description. Good job documenting the specific changes.
examples/cli-drizzle/indexers/2-starknet.indexer.ts (1)
31-33: Implementation of the new idColumn map using wildcard looks correctThe change from a simple string
"_id"to a map with a wildcard key"*"matches the new functionality described in the PR. This demonstrates the new support for idColumn mapping with wildcard support.Note that this change maintains backward compatibility while adding flexibility for specifying different ID columns per table if needed in the future.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/cli/src/cli/common.ts (2)
16-19: Provide clarity on async usage.
Since this function relies onawait renderUsage(cmd), having it beasyncis consistent. However, consider confirming whether any additional asynchronous operations may occur in the future, or if a synchronous approach would suffice.
35-40: Avoid forcing process termination inside a utility function.
While terminating the process on unknown arguments is valid for a CLI, consider surfacing an error or throwing an exception instead. This approach allows the caller to decide whether to exit or handle errors differently, improving reusability and testability.- process.exit(1); + throw new Error(`Unknown arguments: ${wrongArgs.join(", ")}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli/src/cli/common.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
packages/cli/src/cli/common.ts (3)
1-7: Good organization of imports.
The imports from"citty"andconsolaare neatly structured and make it clear which utilities are being used.
20-31: Well-defined logic for collecting defined arguments.
This loop cleanly accounts for both main argument names and aliases. Ensure that any future additions tocmd.argsremain fully compatible with this approach.
32-34: Ensure underscore key usage is consistent.
Filtering out_is a common practice since libraries likeminimistoften store positional args under_. Confirm that no other special keys need to be excluded.Would you like me to run a quick command to verify the usage of
_or other special keys across the CLI?
fracek
left a comment
fracek
left a comment
There was a problem hiding this comment.
Only a couple of tiny things to change.
2 participants
drizzle-ormversion in template--always-reindexflag is setindexerIdrelation to KV StoreidColumnmap support for table specific id column with wildcard support.