-
Notifications
You must be signed in to change notification settings - Fork 524
Add environment variable configuration for BatchAsyncContainerExecutor MaxOperationsInDirectModeBatchRequest #5295
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: master
Are you sure you want to change the base?
Conversation
…atchRequest Co-authored-by: kirankumarkolli <[email protected]>
| /// If the environment variable is not set, the default value from Constants.MaxOperationsInDirectModeBatchRequest is used. | ||
| /// </summary> | ||
| /// <returns>The maximum number of operations allowed in a direct mode batch request.</returns> | ||
| public static int GetMaxOperationsInDirectModeBatchRequest() |
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.
Move to ConfigurationManager as method
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.
Moved the logic to ConfigurationManager.GetMaxOperationsInDirectModeBatchRequest() method as requested. The BatchConfiguration class now delegates to this centralized configuration method. (8abde21)
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.
Remove BatchConfiguration class, its unnecessary
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.
Removed the unnecessary BatchConfiguration class as requested. The logic is now handled directly by ConfigurationManager.GetMaxOperationsInDirectModeBatchRequest() and the wrapper class has been eliminated. Tests have been updated to test ConfigurationManager directly. (00ea126)
| return Constants.MaxOperationsInDirectModeBatchRequest; | ||
| } | ||
|
|
||
| if (int.TryParse(environmentValue, out int parsedValue)) |
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.
Valid only if value is less than Constants.MaxOperationsInDirectModeBatchRequest and positive
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.
Added validation to ensure the value is positive and less than or equal to Constants.MaxOperationsInDirectModeBatchRequest. Updated tests accordingly. (8abde21)
…ound validation Co-authored-by: kirankumarkolli <[email protected]>
…ger directly Co-authored-by: kirankumarkolli <[email protected]>
| container, | ||
| cosmosClientContext, | ||
| Constants.MaxOperationsInDirectModeBatchRequest, | ||
| ConfigurationManager.GetMaxOperationsInDirectModeBatchRequest(), |
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.
Its reading everytime and checking ENVIRONMENT variables which is expensive.
Please read it once and use afterewards
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.
Implemented caching to read environment variable only once using Lazy<T>. The value is now cached on first access and reused for subsequent calls, eliminating the performance overhead. Added test-friendly controls to disable caching during unit tests. (d515dd8)
| /// A read-only string containing the environment variable name for configuring the maximum number of operations | ||
| /// allowed in a direct mode batch request. | ||
| /// </summary> | ||
| internal static readonly string MaxOperationsInDirectModeBatchRequest = "COSMOS_MAX_OPERATIONS_IN_DIRECT_MODE_BATCH_REQUEST"; |
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.
| internal static readonly string MaxOperationsInDirectModeBatchRequest = "COSMOS_MAX_OPERATIONS_IN_DIRECT_MODE_BATCH_REQUEST"; | |
| internal static readonly string MaxOperationsInDirectModeBatchRequest = "AZURE_COSMOS_MAX_OPERATIONS_IN_BATCH_REQUEST"; |
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.
Updated environment variable name to "AZURE_COSMOS_MAX_OPERATIONS_IN_BATCH_REQUEST" as suggested and updated corresponding tests. (d515dd8)
Co-authored-by: kirankumarkolli <[email protected]>
Co-authored-by: kirankumarkolli <[email protected]>
This PR adds the ability to customize the
MaxOperationsInDirectModeBatchRequestused byBatchAsyncContainerExecutorthrough environment variable configuration, as requested in the issue.Changes Made
🆕 New Features
COSMOS_MAX_OPERATIONS_IN_DIRECT_MODE_BATCH_REQUESTenvironment variableConstants.MaxOperationsInDirectModeBatchRequestwhen env var is not set📁 Files Added/Modified
BatchConfiguration.cs(New) - Configuration helper class with environment variable supportBatchAsyncContainerExecutorCache.cs- Updated to use configurable value (minimal 1-line change)BatchConfigurationTests.cs(New) - Comprehensive unit testsBatchAsyncContainerExecutorCacheTests.cs- Added integration test🔧 Usage Example
✅ Implementation Details
COSMOS_MAX_OPERATIONS_IN_DIRECT_MODE_BATCH_REQUESTArgumentExceptionwith descriptive messages for invalid values🧪 Testing
Added comprehensive test coverage including:
BatchAsyncContainerExecutorCache🛡️ Quality Assurance
This implementation provides enterprise-ready configurability for batch operations while maintaining the robustness and reliability of the existing codebase.
Fixes #5294.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.