Add logging flags for tuple writes - #534
Conversation
WalkthroughThe changes introduce optional logging for successful and failed tuple operations in the tuple write and delete commands. New command-line flags allow specifying log file paths for successes and failures. Loggers are created as needed, injected via context, and used during tuple processing. Output is suppressed in the CLI if corresponding logs are enabled. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI_User
participant TupleCmd
participant Logger
participant ImportProcessor
CLI_User->>TupleCmd: Run write/delete with --success-log/--failure-log
TupleCmd->>Logger: Create TupleLogger(s) if log flags set
TupleCmd->>ImportProcessor: Call import function with context (includes loggers)
ImportProcessor->>Logger: Log successes/failures during processing
ImportProcessor->>TupleCmd: Return results
TupleCmd-->>CLI_User: Display output (suppress lists if logs enabled)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (4)
internal/tuple/logger.go (4)
63-79: Consider error handling for marshaling operations.The LogSuccess method ignores errors from yaml.Marshal and json.Marshal operations. While these rarely fail for simple structs, consider logging or handling these errors for robustness.
case ".yaml", ".yml": - b, _ := yaml.Marshal(key) - l.writer.Write(b) - l.writer.Write([]byte("---\n")) + b, err := yaml.Marshal(key) + if err != nil { + return // or log the error + } + l.writer.Write(b) + l.writer.Write([]byte("---\n")) default: // json and jsonl - b, _ := json.Marshal(key) - l.writer.Write(append(b, '\n')) + b, err := json.Marshal(key) + if err != nil { + return // or log the error + } + l.writer.Write(append(b, '\n'))
82-98: Consider error handling for marshaling operations.Similar to LogSuccess, the LogFailure method ignores marshaling errors. Consider adding error handling for consistency and robustness.
100-112: Consider error handling for CSV write operations.The writeCSV method ignores errors from w.Write() operations. While CSV write errors are rare, consider handling them for completeness.
if !l.headerWritten { if l.isFailure { - w.Write([]string{"user", "relation", "object", "reason"}) + _ = w.Write([]string{"user", "relation", "object", "reason"}) } else { - w.Write([]string{"user", "relation", "object"}) + _ = w.Write([]string{"user", "relation", "object"}) } l.headerWritten = true }
78-78: Consider performance implications of frequent flushing.The flush() call after each log operation ensures data integrity but may impact performance for high-volume tuple operations. Consider if this level of flushing is necessary or if periodic flushing would be sufficient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/tuple/delete.go(3 hunks)cmd/tuple/tuple.go(1 hunks)cmd/tuple/write.go(3 hunks)internal/tuple/import.go(4 hunks)internal/tuple/import_test.go(3 hunks)internal/tuple/logger.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/tuple/write.go (1)
internal/tuple/logger.go (4)
TupleLogger(18-24)NewTupleLogger(27-46)WithSuccessLogger(121-123)WithFailureLogger(125-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Tests
- GitHub Check: Test Release Process
- GitHub Check: Lints
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (go)
🔇 Additional comments (23)
cmd/tuple/tuple.go (1)
41-42: Clean flag additions for logging functionality.The new persistent flags are properly defined and follow the existing pattern. The implementation enables users to specify file paths for logging successful and failed tuple operations.
internal/tuple/import_test.go (3)
4-4: Necessary context import for updated function signatures.Adding context import to support the updated processWrites and processDeletes function signatures.
27-27: Proper context parameter addition for test.Using context.Background() is appropriate for test scenarios where no specific context behavior is required.
49-49: Consistent context parameter usage in tests.Maintaining consistency with the processWrites test by using context.Background().
cmd/tuple/delete.go (4)
52-68: Proper logger initialization with resource management.The logger creation logic correctly handles error cases, creates loggers only when paths are provided, and uses defer statements for proper cleanup. The error handling and resource management follow Go best practices.
90-93: Clean context manipulation for logger propagation.The context manipulation properly attaches both success and failure loggers to the context, following the established pattern for context value propagation.
105-105: Smart output suppression logic for successful operations.The condition properly suppresses successful tuple output when success logging is enabled, preventing duplicate reporting while respecting the hideImportedTuples flag.
109-109: Consistent output suppression for failed operations.The condition correctly suppresses failed tuple output when failure logging is enabled, maintaining consistency with the success logging approach.
cmd/tuple/write.go (4)
236-254: Consistent logger initialization pattern.The logger creation logic follows the same pattern as the delete command, ensuring consistency across the codebase. Error handling and resource management are properly implemented.
266-267: Proper context preparation for logger propagation.The context manipulation correctly attaches both loggers to the context, following the established pattern used in the delete command.
282-282: Appropriate output suppression for successful operations.The condition properly suppresses successful tuple output when success logging is enabled, maintaining consistency with the delete command's approach.
286-286: Consistent output suppression for failed operations.The condition correctly suppresses failed tuple output when failure logging is enabled, following the same pattern as the delete command.
internal/tuple/logger.go (4)
18-24: Well-designed TupleLogger struct.The struct design is clean and includes all necessary fields for managing file operations, format handling, and state tracking.
27-46: Robust constructor with proper error handling.The constructor properly handles file creation, permission setting (0o600 for security), and state initialization. Error handling and resource cleanup are well implemented.
48-60: Proper resource management with flush and sync.The Close() method ensures data integrity by flushing buffers and syncing to disk before closing. The nil check prevents panics when closing uninitialized loggers.
121-137: Clean context utility implementation.The context utilities follow Go patterns correctly and provide type-safe access to loggers. The private key types prevent key collisions.
internal/tuple/import.go (7)
143-143: LGTM: Context parameter properly passed to function call.The context is correctly passed to
processWritesAndDeletesmaintaining consistency with the updated function signature.
218-219: LGTM: Context parameters consistently passed to processing functions.Both
processWritesandprocessDeletesfunction calls have been updated to include the context parameter, maintaining consistency with their updated signatures.
301-308: LGTM: Function signature properly updated with context parameter.The
processWritesAndDeletesfunction signature has been correctly updated to accept a context parameter, and the context is properly passed to the downstream functions.
319-327: LGTM: Logging implementation follows good practices.The logging implementation is well-structured:
- Loggers are retrieved from context using appropriate helper functions
- Null checks prevent runtime errors when loggers are not available
- Logging occurs after successful tuple processing, ensuring consistency
- The logging call is placed appropriately within the success condition
330-337: LGTM: Error logging implementation is consistent and safe.The failure logging implementation:
- Creates a proper
failedWriteResponsestruct with tuple key and reason- Adds the failed response to the array before logging
- Includes null check for the failure logger
- Maintains consistency with the success logging pattern
353-367: LGTM: Success logging for deletes follows established patterns.The delete success logging implementation:
- Retrieves loggers from context consistently with the write operations
- Includes proper null checks for defensive programming
- Logs the successful delete operation after adding to the successful array
- Maintains consistency with the overall logging approach
370-377: LGTM: Delete failure logging is properly implemented.The delete failure logging implementation:
- Creates a proper
failedWriteResponsestruct for failed deletes- Includes error reason extraction using the existing
extractErrMsgfunction- Performs null check before logging
- Maintains consistency with write failure logging
1 participant
Summary
Testing
make test-unitgo build ./...https://chatgpt.com/codex/tasks/task_e_686c2ec5a4b48322adc0e1ba7263f18e
Summary by CodeRabbit
New Features
Documentation
Tests