Users/ramacg/refactor managed uploader

[ag-ramachandran]

No description provided.

[Copilot]

Pull request overview

This PR refactors the ManagedUploader by simplifying the selectContainers method interface and enhancing the configuration cache to support server-specified refresh intervals.

Key Changes:

• Removed configurationCache parameter from selectContainers method, accessing it as a class property instead
• Changed configurationCache visibility from private to public in ContainerUploaderBase
• Added logic to use server-specified refresh intervals from configuration responses
• Added comprehensive tests for container selection with different upload methods

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ingest-v2/src/test/resources/config-response.json	New test fixture providing sample configuration with both storage and lake containers
ingest-v2/src/test/kotlin/com/microsoft/azure/kusto/ingest/v2/uploader/ManagedUploaderTest.kt	New parameterized test validating container selection logic for DEFAULT, STORAGE, and LAKE upload methods
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/uploader/ManagedUploader.kt	Removed configurationCache parameter from selectContainers method signature
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/uploader/ContainerUploaderBase.kt	Made configurationCache property public and updated selectContainers signature and documentation
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/common/ConfigurationCache.kt	Enhanced to calculate effective refresh interval using minimum of default and server-specified intervals

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Test Results

530 tests  ±0   521 ✅ ±0   3m 25s ⏱️ -1s
 31 suites ±0     9 💤 ±0 
 31 files   ±0     0 ❌ ±0 

Results for commit 11d42f6. ± Comparison against base commit b9ccf14.

[github-actions]

Test Results

530 tests  ±0   521 ✅ ±0   3m 34s ⏱️ +10s
 31 suites ±0     9 💤 ±0 
 31 files   ±0     0 ❌ ±0 

Results for commit a51c91a. ± Comparison against base commit b9ccf14.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The refresh interval parsing uses LocalTime.parse() which expects a time format (HH:mm:ss), but the actual format in the config is a TimeSpan/Duration string ("01:00:00"). While "01:00:00" may parse as LocalTime, this is semantically incorrect - you're parsing a duration as a time-of-day. Consider using Duration.parse() with the ISO-8601 duration format (e.g., "PT1H") or implement proper TimeSpan parsing that handles the format correctly.

[Copilot]

The test logic for DEFAULT upload method has an issue. According to the test comment on lines 40-41, when DEFAULT is used and both lake and storage are available, lake should be preferred. However, the config-response.json sets "preferredUploadMethod": "Lake", and the actual ManagedUploader implementation (lines 64-100 in ManagedUploader.kt) will honor this server preference. The test correctly expects lake, but the comment suggests this is a client-side default rather than respecting the server's preference. Consider clarifying whether this test is validating the server preference mechanism or the client-side default behavior.

Suggested change

	// If we get both lake and storage and user does not specify, lake is preferred. If user
	// specifies, respect that.
	// When the server configuration prefers Lake and the user does not specify (DEFAULT),
	// ManagedUploader should honor the server preference and use Lake. If the user explicitly
	// specifies a method (e.g., STORAGE), that explicit choice is respected.