Hbase Simple SQL One Value fix#4016
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested reviewers
Poem
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 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: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs(7 hunks)
🔇 Additional comments (1)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs (1)
913-917:
Fix incorrect initialization of 'columnList'
The columnList is incorrectly initialized with []. Initialize it using new List<string>();.
Apply this diff to correct the initialization:
- List<string> columnList = [];
+ List<string> columnList = new List<string>();Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs (2)
419-423: Consider using LINQ for column name processing.The current implementation can be simplified using LINQ.
- List<string> columnNameList = []; - foreach (string col in selectedcols) - { - columnNameList.Add(col.Trim()); - } + List<string> columnNameList = selectedcols.Select(col => col.Trim()).ToList();
451-464: Improve data validation and error handling.Good practice to throw an exception when no data is found, but consider:
- Adding more context to the error message
- Using a custom exception type for better error handling
- throw new InvalidDataException("Data not found"); + throw new InvalidDataException($"No data found for table '{table}' with the specified criteria");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs(7 hunks)
🧰 Additional context used
📓 Learnings (1)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs (2)
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4016
File: Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs:874-890
Timestamp: 2024-12-04T08:55:11.460Z
Learning: The `GetTableList` method in `Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs` is synchronous, so `await` cannot be used unless the method is changed to be async.
Learnt from: GokulBothe99
PR: Ginger-Automation/Ginger#4016
File: Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs:419-419
Timestamp: 2024-12-04T08:49:40.756Z
Learning: In the `GingerHbase` class of `Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs`, it's acceptable to initialize lists using `[]` syntax.
🔇 Additional comments (3)
Ginger/GingerCoreNET/Database/NoSqlBase/GingerHbase.cs (3)
352-355: LGTM! Field declarations are well-structured.
The new fields are appropriately scoped and named, providing clear intent for their usage in data retrieval and processing.
401-406: 🛠️ Refactor suggestion
Improve error handling for table schema retrieval.
The code properly validates column families but uses blocking .Result call. Consider using ConfigureAwait(false) to prevent potential deadlocks.
- var familyNameList = actionClient.GetTableSchemaAsync(table, null).Result.columns.ToList();
+ var familyNameList = (await actionClient.GetTableSchemaAsync(table, null).ConfigureAwait(false)).columns.ToList();Likely invalid or redundant comment.
921-948: 🛠️ Refactor suggestion
Improve async implementation in GetColumnList.
The method is marked async but still uses blocking calls with Task.Run and .Result.
- Task getTablesTask = Task.Run(() =>
- {
- try
- {
- scanInfo = actionClient.CreateScannerAsync(Tablename, scanner, requestOption).Result;
- while ((next = actionClient.ScannerGetNextAsync(scanInfo, requestOption).Result) != null)
- {
- // ... processing ...
- }
- }
- catch (Exception ex)
- {
- Reporter.ToLog(eLogLevel.WARN, "Unable to connect to Hbase and get Column list ", ex);
- }
- });
- getTablesTask.Wait();
+ try
+ {
+ scanInfo = await actionClient.CreateScannerAsync(Tablename, scanner, requestOption).ConfigureAwait(false);
+ while ((next = await actionClient.ScannerGetNextAsync(scanInfo, requestOption).ConfigureAwait(false)) != null)
+ {
+ // ... processing ...
+ }
+ }
+ catch (Exception ex)
+ {
+ Reporter.ToLog(eLogLevel.WARN, "Unable to connect to Hbase and get Column list ", ex);
+ }Likely invalid or redundant comment.
2 participants
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor