Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changelog upload service no longer skips ArtifactType.Bundle early and instead routes bundle uploads through the common Upload(...) flow. Directory resolution is artifact-type-specific (ResolveBundleDirectory vs ResolveChangelogDirectory). Target discovery now uses DiscoverBundleUploadTargets for bundles (producing per-product S3 keys under {product}/bundles/{fileName}) and DiscoverUploadTargets for changelogs. Bundles are parsed via ReadProductsFromBundle (deserializing bundle YAML and extracting product IDs) with validation and warnings on errors; discovered targets are uploaded to S3 via the existing uploader. Sequence DiagramsequenceDiagram
participant Client
participant ChangelogUploadService
participant DirectoryResolver
participant BundleDiscovery
participant BundleDeserializer
participant ProductValidator
participant S3Uploader
Client->>ChangelogUploadService: Upload(args with ArtifactType=Bundle)
activate ChangelogUploadService
ChangelogUploadService->>DirectoryResolver: ResolveBundleDirectory(args, config)
activate DirectoryResolver
DirectoryResolver-->>ChangelogUploadService: bundleDirectory
deactivate DirectoryResolver
ChangelogUploadService->>BundleDiscovery: DiscoverBundleUploadTargets(bundleDirectory)
activate BundleDiscovery
BundleDiscovery->>BundleDiscovery: Find top-level *.yaml/*.yml files
BundleDiscovery->>BundleDeserializer: ReadProductsFromBundle(bundleFile)
activate BundleDeserializer
BundleDeserializer->>BundleDeserializer: Deserialize YAML, extract distinct ProductIds
BundleDeserializer-->>BundleDiscovery: productIds[]
deactivate BundleDeserializer
BundleDiscovery->>ProductValidator: Validate product names (ProductNameRegex)
activate ProductValidator
ProductValidator-->>BundleDiscovery: valid/invalid
deactivate ProductValidator
BundleDiscovery->>BundleDiscovery: Emit UploadTarget per valid product (S3Key = {product}/bundles/{fileName})
BundleDiscovery-->>ChangelogUploadService: UploadTarget[]
deactivate BundleDiscovery
ChangelogUploadService->>S3Uploader: PutObjectAsync(uploadTargets)
activate S3Uploader
S3Uploader-->>ChangelogUploadService: Upload responses
deactivate S3Uploader
ChangelogUploadService-->>Client: Result/diagnostics
deactivate ChangelogUploadService
🚥 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✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs (1)
142-184: Consider extracting shared logic withDiscoverUploadTargets.This method is nearly identical to
DiscoverUploadTargets(lines 98-140), differing only in the product extraction call and S3 key path segment (bundlesvschangelogs). A shared helper could reduce duplication, but it's not critical given the slight behavioral differences.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs` around lines 142 - 184, DiscoverBundleUploadTargets duplicates DiscoverUploadTargets; extract a shared helper to remove duplication by factoring the common file enumeration, symlink validation (SymlinkValidator.ValidateFileAccess), product name validation (ProductNameRegex), and UploadTarget creation, and pass the differing pieces (product extraction function ReadProductsFromBundle vs existing extractor and the S3 segment string "bundles" vs "changelogs") as parameters (e.g., Func<string,IReadOnlyList<string>> productExtractor and string s3Segment). Replace DiscoverBundleUploadTargets and DiscoverUploadTargets to call the new helper, keeping UploadTarget creation and filePath/fileName logic centralized and preserving the collector warnings and logger usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs`:
- Around line 186-204: ReadProductsFromBundle currently assumes
ReleaseNotesSerialization.DeserializeBundle(content) returns a non-null bundle
and accesses bundle.Products directly, relying on exceptions to handle nulls;
update ReadProductsFromBundle to check for nulls like ReadProductsFromFragment
does: after deserializing (ReleaseNotesSerialization.DeserializeBundle) verify
if bundle == null or bundle.Products == null and return new List<string>()
early, then proceed with the LINQ (Select p => p.ProductId ...
Distinct().ToList()); keep the existing catch/logging behavior but replace the
invalid [] return with new List<string>().
---
Nitpick comments:
In `@src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cs`:
- Around line 142-184: DiscoverBundleUploadTargets duplicates
DiscoverUploadTargets; extract a shared helper to remove duplication by
factoring the common file enumeration, symlink validation
(SymlinkValidator.ValidateFileAccess), product name validation
(ProductNameRegex), and UploadTarget creation, and pass the differing pieces
(product extraction function ReadProductsFromBundle vs existing extractor and
the S3 segment string "bundles" vs "changelogs") as parameters (e.g.,
Func<string,IReadOnlyList<string>> productExtractor and string s3Segment).
Replace DiscoverBundleUploadTargets and DiscoverUploadTargets to call the new
helper, keeping UploadTarget creation and filePath/fileName logic centralized
and preserving the collector warnings and logger usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2115e415-e8ad-4c94-9eab-7713aeb89dd5
📒 Files selected for processing (2)
src/services/Elastic.Changelog/Uploading/ChangelogUploadService.cstests/Elastic.Changelog.Tests/Uploading/ChangelogUploadServiceTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unblocks elastic/docs-actions#42
This pull request adds support for uploading "bundle" artifacts in the changelog upload service. It introduces logic to discover and upload bundle YAML files, validates product names, and updates the tests to cover this new functionality. The changes also ensure that multiple products in a bundle are handled correctly, and invalid product names are skipped with appropriate warnings.
Bundle artifact upload support:
DiscoverBundleUploadTargetsto find bundle YAML files, validate product names, and create an upload target for each valid product in the bundle. Invalid product names are skipped with a warning.ReadProductsFromBundleto extract product IDs from bundle files.ResolveBundleDirectoryto determine the correct directory for bundle artifacts, supporting both explicit and configuration-based paths.Upload logic enhancements:
Uploadmethod to support the new bundle artifact type: resolves the correct directory, discovers bundle upload targets, and logs details using the artifact type.Test coverage improvements: