-
Notifications
You must be signed in to change notification settings - Fork 2
FSPW-385 - Frends.AmazonS3.DownloadObject - Version 3.0.0 #45
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
base: main
Are you sure you want to change the base?
Conversation
…put and connection parameters Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
… DestinationDirectory Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
…d PreSignedUrl Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
…tion Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
…r test isolation Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
Co-authored-by: aider (anthropic/claude-sonnet-4-20250514) <[email protected]>
WalkthroughThe codebase underwent a major refactor of the Frends.AmazonS3.DownloadObject task, splitting parameters into three classes—Input, Connection, and Options—while improving naming consistency and error handling. The workflow files were updated to enable strict analyzers, and the target framework was upgraded to .NET 8.0. Tests and documentation were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrendsTask
participant S3Client
participant ErrorHandler
User->>FrendsTask: DownloadObject(Input, Connection, Options, CancellationToken)
FrendsTask->>S3Client: Authenticate (via Connection)
FrendsTask->>S3Client: List and filter S3 objects (Input)
loop For each matching object
FrendsTask->>S3Client: Download object
FrendsTask->>FrendsTask: Write to TargetDirectory (Options)
end
alt Error occurs
FrendsTask->>ErrorHandler: Handle(exception, Options, results)
ErrorHandler-->>FrendsTask: Result with Error or throw exception
end
FrendsTask-->>User: Result (Objects, Success, Error)
Suggested reviewers
Poem
✨ 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 (
|
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: 1
🧹 Nitpick comments (2)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Options.cs (2)
33-39:FileLockedRetriesis a retry count, not “value in seconds”.The implementation loops
<= FileLockedRetriestimes with a fixed 1 s sleep. Either change the XML comment to reflect “number of retries” or refactor the code to accept seconds and convert internally.
48-53: PreventnullinErrorMessageOnFailure.
stringdefaults tonull; many callers expect empty string. Initialise tostring.Emptyto avoid extra null-checks.- public string ErrorMessageOnFailure { get; set; } + public string ErrorMessageOnFailure { get; set; } = string.Empty;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/DownloadObject_build_and_test_on_main.yml(1 hunks).github/workflows/DownloadObject_build_and_test_on_push.yml(2 hunks).github/workflows/DownloadObject_release.yml(1 hunks)Frends.AmazonS3.DownloadObject/CHANGELOG.md(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.Tests/Frends.AmazonS3.DownloadObject.Tests.csproj(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.Tests/UnitTests.cs(13 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Enums.cs(2 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Error.cs(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Input.cs(2 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Options.cs(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Result.cs(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/DownloadObject.cs(7 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.csproj(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Helpers/ErrorHandler.cs(1 hunks)Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/migration.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
Frends.*/Frends.*/*.csproj
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
Frends.*/Frends.*.Tests/*
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
Frends.*/**/*.cs
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
Frends.*/Frends.*/*.cs
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
Frends.*/CHANGELOG.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (3)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Error.cs (1)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/DownloadObject.cs (1)
AmazonS3(22-220)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Options.cs (1)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/DownloadObject.cs (1)
AmazonS3(22-220)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Input.cs (1)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/DownloadObject.cs (1)
AmazonS3(22-220)
🪛 Gitleaks (8.27.2)
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Input.cs
71-71: Identified a pattern that may indicate AWS credentials, risking unauthorized cloud resource access and data breaches on AWS platforms.
(aws-access-token)
🪛 LanguageTool
Frends.AmazonS3.DownloadObject/CHANGELOG.md
[grammar] ~45-~45: Ensure spelling is correct
Context: ...s result.Data to use result.Objects instead 5. Error handling: Consider setting `Thr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~53-~53: Ensure spelling is correct
Context: ...ility and testability with better class structure ## [2.2.0] - 2024-12-11 ### Updated - List...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_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). (3)
- GitHub Check: Analyze (csharp)
- GitHub Check: build / Check Code Quality
- GitHub Check: build / Build on windows-latest
🔇 Additional comments (26)
.github/workflows/DownloadObject_build_and_test_on_push.yml (1)
16-16: Verify that the reusable workflow actually exposesstrict_analyzers.
strict_analyzers: truewill be ignored – and the job will fail – ifbuild_test.ymldoes not declare that input. Confirm the input exists (and is spelled identically) inFrendsPlatform/FrendsTasks/.github/workflows/build_test.yml.Also applies to: 25-25
.github/workflows/DownloadObject_build_and_test_on_main.yml (1)
16-16: Same validation needed for main-branch workflow.Again, make sure the called workflow supports the
strict_analyzersinput; otherwise the build onmainwill break right after merge.Also applies to: 24-24
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.Tests/Frends.AmazonS3.DownloadObject.Tests.csproj (1)
4-4: Confirm test-tooling compatibility with .NET 8.
Microsoft.NET.Test.Sdk16.11.0 predates .NET 8 and may fail to discover tests. Upgrade to 17.9+ to stay in the supported matrix..github/workflows/DownloadObject_release.yml (1)
11-11: Release workflow needs the same input-compatibility check.Ensure
FrendsPlatform/FrendsTasks/.github/workflows/release.ymlacceptsstrict_analyzers; otherwise the publish job will error out.Also applies to: 19-19
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.csproj (4)
1-2: LGTM: Proper XML declaration and SDK configurationThe explicit XML declaration with UTF-8 encoding and Microsoft.NET.Sdk usage follows best practices.
4-5: LGTM: Framework upgrade and version bump are appropriateThe upgrade to .NET 8.0 complies with the coding guidelines, and the major version bump to 3.0.0 is appropriate for the significant refactoring.
6-15: LGTM: All required metadata fields are present and correctly configuredThe project file includes all required fields per the coding guidelines: Version, Authors (Frends), Description, RepositoryUrl, GenerateDocumentationFile (true), and PackageLicenseExpression (MIT).
27-30: LGTM: Appropriate content files for major version releaseThe inclusion of migration.json and CHANGELOG.md in the package root is appropriate for a major version release with breaking changes.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Enums.cs (1)
43-48: LGTM: Improved naming consistency for abbreviationsThe renaming of
AWSCredentialstoAwsCredentialsandPreSignedURLtoPreSignedUrlfollows Microsoft C# naming conventions for abbreviations, as specified in the coding guidelines.Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Helpers/ErrorHandler.cs (2)
7-11: LGTM: Well-structured static error handling classThe class is properly declared as static with comprehensive XML documentation and follows Microsoft C# naming conventions.
12-31: LGTM: Comprehensive error handling implementationThe method provides robust error handling with proper documentation, clear conditional logic for throw vs. return scenarios, and structured error information including stack trace and inner exception details.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/migration.json (3)
1-8: LGTM: Well-structured migration metadataThe migration file header properly identifies the task, version, and provides a clear description of the major refactoring.
9-63: LGTM: Comprehensive parameter restructuringThe migration systematically reorganizes parameters into logical classes (Input, Connection, Options) and improves naming consistency. The parameter renaming (e.g., S3Directory → SourceDirectory) enhances clarity.
64-73: LGTM: Well-designed error handling optionsThe new error handling parameters (ThrowErrorOnFailure, ErrorMessageOnFailure) have sensible defaults that maintain backward compatibility while enabling enhanced error handling.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Error.cs (2)
6-18: LGTM: Well-designed error properties with excellent documentationThe Error class properties are appropriately typed with comprehensive XML documentation and helpful examples. The use of dynamic for AdditionalInfo provides necessary flexibility for error context.
20-38: LGTM: Excellent constructor designThe Error class provides both parameterized and parameterless constructors with proper documentation, sensible defaults, and flexible initialization options.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Result.cs (1)
8-34: LGTM!The Result class properly follows the Frends task result structure requirements with Success, Objects (task-specific return value), and Error properties. The implementation is clean and well-documented.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject.Tests/UnitTests.cs (2)
22-58: Test initialization correctly implements the new parameter structure.The setup properly initializes all three parameter objects (Input, Connection, Options) with appropriate default values.
68-586: Comprehensive test coverage for the refactored implementation.The tests properly cover:
- Pre-signed URL and AWS credentials authentication methods
- All destination file exists actions (Overwrite, Info, Error)
- New error handling options (ThrowErrorOnFailure, ErrorMessageOnFailure)
- Result structure with Objects and Error properties
Test naming follows Microsoft conventions and provides good coverage of edge cases.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/Definitions/Input.cs (2)
9-45: Input class properly separates download-specific parameters.The class structure follows the documented refactoring with clear property names and appropriate XML documentation.
50-95: Connection class correctly implements AWS authentication parameters.The property naming follows Microsoft C# conventions (AwsCredentials, PreSignedUrl) and includes proper UI hints for conditional display.
Frends.AmazonS3.DownloadObject/CHANGELOG.md (1)
1-54: Comprehensive changelog documentation for version 3.0.0.The changelog properly follows the Keep a Changelog format and provides:
- Clear documentation of breaking changes
- Detailed parameter migration mappings
- Step-by-step migration guide
- Technical improvements summary
All documented changes align with the actual code modifications.
Frends.AmazonS3.DownloadObject/Frends.AmazonS3.DownloadObject/DownloadObject.cs (4)
26-35: Method signature correctly implements the new parameter structure.The XML documentation properly describes all parameters and the complete result structure.
40-88: Download logic correctly uses the refactored parameter structure.All property references have been properly updated to use the correct parameter objects (input, connection, options).
94-97: Proper implementation of centralized error handling.The error handling correctly uses the new ErrorHandler class with options-based configuration.
100-167: Helper methods correctly use the Options parameter.The WriteToFile and FileLocked methods properly reference options for file handling behavior and retry configuration.
Changes
Changes for FSPW-385 - Frends.AmazonS3.DownloadObject
Summary
Review Checklist
1. Frends Task Project Files
Frends.*/Frends.*/*.csproj<PackageLicenseExpression>MIT</PackageLicenseExpression>)<Version><Authors>Frends</Authors><Description><RepositoryUrl><GenerateDocumentationFile>true</GenerateDocumentationFile>2. File: FrendsTaskMetadata.json
Frends.*/Frends.*/FrendsTaskMetadata.json3. File: README.md
Frends.*/README.md4. File: CHANGELOG.md
Frends.*/CHANGELOG.md5. File: migration.json
Frends.*/Frends.*/migration.json6. Source Code Documentation
Frends.*/Frends.*/*.cs<summary>XML comments<example>XML comments<frendsdocs>XML comments, if needed7. GitHub Actions Workflows
.github/workflows/*.yml*_test.yml*_main.yml*_release.yml8. Task Result Object Structure
Frends.*/Frends.*/*.csSuccess(bool)Additional Notes
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
Chores