Fix EventHubs Tool Issues.#2678
Draft
g2vinay wants to merge 1 commit into
Draft
Conversation
- EH-07: DeleteNamespaceAsync returns false (not true) for 404
- EH-01: NamespaceDeleteCommand shows correct not-found message when success=false
- EH-02: GetNamespacesAsync uses subscriptionResource.GetEventHubsNamespacesAsync()
instead of N+1 per-resource-group calls
- EH-03: GetNamespaceAsync validates namespaceName and resourceGroup parameters
- EH-04: Invalid SKU tier throws ArgumentException instead of silently defaulting to Standard
- EH-06: EventHubUpdateCommand passes status option to CreateOrUpdateEventHubAsync;
service applies EventHubEntityStatus with validation
- EH-08: CreateOrUpdateConsumerGroupAsync adds null guards on resourceGroup,
namespace and eventHub lookups; throws KeyNotFoundException on missing parent
- EH-09: EventHubGetCommand error log messages corrected (were swapped)
- EH-11: Not-found resource paths throw KeyNotFoundException (HTTP 404)
instead of InvalidOperationException (HTTP 500)
- EH-13: EventHubsService is now sealed
Tests: added ExecuteAsync_PassesStatusOptionToService and
ExecuteAsync_WhenNamespaceNotFound_ReturnsNotFoundMessage;
updated all existing mocks to match new service signatures
| "STANDARD" => EventHubsSkuTier.Standard, | ||
| "PREMIUM" => EventHubsSkuTier.Premium, | ||
| null or "" => EventHubsSkuTier.Standard, | ||
| _ => throw new ArgumentException( |
Contributor
There was a problem hiding this comment.
This will need to be listed as a breaking change
| { | ||
| _logger.LogInformation( | ||
| "Event Hubs namespace '{NamespaceName}' not found in resource group '{ResourceGroup}'", | ||
| _logger.LogWarning( |
Contributor
There was a problem hiding this comment.
I don't think this warrants being a warning level log.
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
2 participants
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.
Fix EventHubs bugs: correctness, reliability, and performance
What changed
Correctness
DeleteNamespaceAsyncnow returnsfalsefor 404 instead oftrue.NamespaceDeleteCommandreports "not found" rather than "deleted successfully" when the namespace never existed.EventHubUpdateCommand—--statusoption is now forwarded to the service and applied. Previously it was accepted but silently discarded, leaving the hub unchanged.ArgumentExceptioninstead of silently defaulting toStandard.Reliability
CreateOrUpdateConsumerGroupAsyncnow validates parent resources (resource group → namespace → event hub) before proceeding, throwingKeyNotFoundExceptionwith an actionable message at each level instead of an unhandledNullReferenceException.GetNamespaceAsyncnow validatesnamespaceNameandresourceGroupbefore making any ARM call.KeyNotFoundException→ HTTP 404, notInvalidOperationException→ HTTP 500.Performance
GetNamespacesAsync(no resource group filter) replaced an N+1 per-resource-group loop with a singleGetEventHubsNamespacesAsyncsubscription-scoped call.Polish
EventHubGetCommanderror log messages were swapped between the single-get and list-all paths — corrected.EventHubsServiceis nowsealed.