fix unloading #71
fix unloading #71
Conversation
WalkthroughAdded static assembly unload handling to the MicrosoftSQL plugin (static constructor subscribes to AssemblyLoadContext.Unloading; handler detaches itself), bumped project version to 2.2.0, and added a changelog entry for the fix. Minor formatting changes in two enum files. Changes
Sequence Diagram(s)sequenceDiagram
participant Host
participant AssemblyLoadContext
participant MicrosoftSQL
Host ->> MicrosoftSQL: Load plugin (type init)
MicrosoftSQL ->> AssemblyLoadContext: Subscribe to Unloading event (static ctor)
Note over MicrosoftSQL,AssemblyLoadContext: Plugin active
AssemblyLoadContext ->> Host: Begin unload
AssemblyLoadContext -->> MicrosoftSQL: Unloading event fired
MicrosoftSQL ->> AssemblyLoadContext: OnPluginUnloadingRequested -> Unsubscribe
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs`:
- Line 38: The Result class for ExecuteQueryToFile is missing the required
Success bool and Error object; update the ExecuteQueryToFile.Result type to
include a public bool Success property and an Error property (object/class) with
Message and AdditionalInfo fields, while keeping existing task-specific
properties (EntriesWritten, Path, FileName); mirror the shape used by
ExecuteQuery.Result (or reuse its Error type) so callers can check Success and
read Error.Message/Error.AdditionalInfo when failures occur.
In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj`:
- Line 6: The Result class in Definitions/Result.cs is missing the public
properties referenced by the pipeline; add public bool Success { get; set; } and
public string Error { get; set; } to the Result class (ensure they are public
auto-properties on the Result type so existing callers/serialization continue to
work).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.mdFrends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csFrends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj
🧰 Additional context used
📓 Path-based instructions (4)
Frends.*/CHANGELOG.md
⚙️ CodeRabbit configuration file
Frends.*/CHANGELOG.md: Validate format against Keep a Changelog (https://keepachangelog.com/en/1.0.0/)
Include all functional changes and indicate breaking changes with upgrade notes.
Avoid notes like "refactored xyz" unless it affects functionality.
Files:
Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md
Frends.*/Frends.*/*.cs
⚙️ CodeRabbit configuration file
Frends.*/Frends.*/*.cs: Ensure every public method and class:
- Has
and XML comments
- If the documentation is very long then it can also use element
- Follows Microsoft C# code conventions
- Uses semantic task result documentation (e.g., Success, Error, Data)
Frends.*/Frends.*/*.cs: Validate all task result classes include:
- Success (bool)
- Task-specific return value (e.g. Data, FilePaths)
- Error object with Message and AdditionalInfo
- Ensure result structure is flat, simple, and avoids 3rd-party types.
- Use dynamic or JToken only when the structure is unknown.
Files:
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs
Frends.*/**/*.cs
⚙️ CodeRabbit configuration file
Frends.*/**/*.cs: Code must follow Microsoft C# coding standards, including:
- PascalCase for public members and task parameters
- Proper naming for abbreviations (Csv, Url, Api)
- Use of var only when type is obvious
- Clean structure and no unused code
Files:
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs
Frends.*/Frends.*/*.csproj
⚙️ CodeRabbit configuration file
Frends.*/Frends.*/*.csproj: Ensure the .csproj targets .NET 6 or 8, uses the MIT license, and includes the following fields:
= Frends
= true
= MIT
Follow Microsoft C# project file conventions.
Files:
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj
🪛 GitHub Actions: ExecuteQueryToFile_build_test
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs
[error] 38-38: FT0011: Class should include a 'Success' property
[error] 38-38: FT0011: Class should include a 'Error' property
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj
[error] 38-38: FT0011: Class should include a 'Success' property
[error] 38-38: FT0011: Class should include a 'Error' property
⏰ 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). (1)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (3)
Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md (1)
3-5: LGTM!The changelog entry follows the Keep a Changelog format correctly with proper versioning, date format, and categorization under "Fixed".
Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs (2)
19-28: Static constructor pattern for assembly unload subscription looks correct.The null check on
currentContextis appropriate, and subscribing toUnloadingin a static constructor is a valid pattern for plugin lifecycle management.
84-87: Verify the unload handler implementation is complete.The handler only unsubscribes itself from the
Unloadingevent but performs no actual resource cleanup. If the goal is to "fix unloading assembly" issues, this handler may be missing cleanup logic (e.g., disposing connections, clearing caches, releasing native resources).Is this intentional? If the fix requires only removing the event subscription to prevent the assembly from being held in memory, this is sufficient. However, if there are other resources that need cleanup during unload, they should be added here.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /// <param name="options">Options parameters.</param> | ||
| /// <param name="cancellationToken">Cancellation token given by Frends.</param> | ||
| /// <returns>Object { int EntriesWritten, string Path, string FileName }</returns> | ||
| public static async Task<Result> ExecuteQueryToFile([PropertyTab] Input input, [PropertyTab] Options options, CancellationToken cancellationToken) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the Result class definition
ast-grep --pattern 'class Result {
$$$
}'Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 60
🏁 Script executed:
#!/bin/bash
# Search for Result class definition
rg "class Result|record Result|struct Result" -t cs --max-count 20Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 638
🏁 Script executed:
#!/bin/bash
# Read the Result class definition
cat -n "Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Definitions/Result.cs"Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 1183
🏁 Script executed:
#!/bin/bash
# Check another Result class to compare
cat -n "Frends.MicrosoftSQL.ExecuteQuery/Frends.MicrosoftSQL.ExecuteQuery/Definitions/Result.cs"Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 1825
Result class missing required Success and Error properties.
The Result class is missing required properties per the coding guidelines. It currently has task-specific return values (EntriesWritten, Path, FileName) but lacks:
Success(bool) — required for all task result classesErrorobject withMessageandAdditionalInfo— required for error handling
Compare with ExecuteQuery.Result which correctly implements Success, ErrorMessage, and task-specific data. Align ExecuteQueryToFile.Result to match this pattern.
🧰 Tools
🪛 GitHub Actions: ExecuteQueryToFile_build_test
[error] 38-38: FT0011: Class should include a 'Success' property
[error] 38-38: FT0011: Class should include a 'Error' property
🤖 Prompt for AI Agents
In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs`
at line 38, The Result class for ExecuteQueryToFile is missing the required
Success bool and Error object; update the ExecuteQueryToFile.Result type to
include a public bool Success property and an Error property (object/class) with
Message and AdditionalInfo fields, while keeping existing task-specific
properties (EntriesWritten, Path, FileName); mirror the shape used by
ExecuteQuery.Result (or reuse its Error type) so callers can check Success and
read Error.Message/Error.AdditionalInfo when failures occur.
| <TargetFramework>net6.0</TargetFramework> | ||
| <LangVersion>Latest</LangVersion> | ||
| <Version>2.1.0</Version> | ||
| <Version>2.2.0</Version> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Find and display the full .csproj file
echo "=== Frends.MicrosoftSQL.ExecuteQueryToFile.csproj content ==="
find . -name "Frends.MicrosoftSQL.ExecuteQueryToFile.csproj" -type f -exec cat {} \;Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 2042
🏁 Script executed:
#!/bin/bash
# Step 2: Find the Result class definition
echo "=== Searching for Result class definition ==="
find . -path "*/Frends.MicrosoftSQL.ExecuteQueryToFile/*" -name "*.cs" -type f | head -20Repository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 1798
🏁 Script executed:
#!/bin/bash
cat ./Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Definitions/Result.csRepository: FrendsPlatform/Frends.MicrosoftSQL
Length of output: 931
Version bump and .csproj configuration look correct.
The version increment from 2.1.0 to 2.2.0 follows semantic versioning, and all .csproj requirements are met:
- TargetFramework: net6.0 ✓
- Version, Authors (Frends), Description, RepositoryUrl all present ✓
- PackageLicenseExpression: MIT ✓
- GenerateDocumentationFile: true ✓
However, the Result class in Definitions/Result.cs is missing the required Success and Error properties that are referenced in the pipeline failures (FT0011). Add these properties to the Result class definition before merging.
🤖 Prompt for AI Agents
In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj`
at line 6, The Result class in Definitions/Result.cs is missing the public
properties referenced by the pipeline; add public bool Success { get; set; } and
public string Error { get; set; } to the Result class (ensure they are public
auto-properties on the Result type so existing callers/serialization continue to
work).
Review Checklist
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.