-
Notifications
You must be signed in to change notification settings - Fork 45
Feature/add ingestion source blob #440
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: feature/IngestV2
Are you sure you want to change the base?
Feature/add ingestion source blob #440
Conversation
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/client/IngestionOperation.kt
Outdated
Show resolved
Hide resolved
.../kotlin/com/microsoft/azure/kusto/ingest/v2/common/serialization/OffsetDateTimeSerializer.kt
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/common/IngestionMethod.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/client/IngestProperties.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/QueuedIngestionApiWrapper.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/QueuedIngestionApiWrapper.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/QueuedIngestionApiWrapper.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/main/kotlin/com/microsoft/azure/kusto/ingest/v2/QueuedIngestionClient.kt
Show resolved
Hide resolved
ingest-v2/src/test/kotlin/com/microsoft/azure/kusto/ingest/v2/ConfigurationApiWrapperTest.kt
Outdated
Show resolved
Hide resolved
ingest-v2/src/test/kotlin/com/microsoft/azure/kusto/ingest/v2/ConfigurationApiWrapperTest.kt
Outdated
Show resolved
Hide resolved
a62a644 to
75c6545
Compare
8239293 to
058226c
Compare
c4b66b0 to
ffe3cda
Compare
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.
Pull Request Overview
This PR implements Azure Kusto ingestion client v2 with comprehensive testing infrastructure. It introduces new client classes for streaming and queued ingestion operations, configuration management, and supporting utilities for data ingestion to Azure Data Explorer.
- Adds streaming and queued ingestion clients with blob-based and direct ingestion support
- Implements configuration client for cluster configuration retrieval
- Creates comprehensive test suites for all client types with end-to-end validation
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| StreamingIngestClient.kt | New client for direct and blob-based streaming ingestion operations |
| QueuedIngestionClient.kt | New client for queued ingestion with polling and status tracking |
| ConfigurationClient.kt | Replaces ConfigurationApiWrapper with improved error handling |
| ConfigurationClientTest.kt | Updated test with parallel execution support and better test structure |
| StreamingIngestClientTest.kt | New test suite for streaming ingestion with parameterized test cases |
| QueuedIngestionClientTest.kt | New test suite for queued ingestion with mapping validation |
| IngestV2TestBase.kt | New base test class with common setup for E2E tests |
| IngestClient.kt | New interface with common response handling for ingestion clients |
| KustoBaseApiClient.kt | Enhanced with serialization support, timeout configuration, and improved error handling |
| openapi.yaml | Updated API specification with additional headers and renamed mapping properties |
| pom.xml | Updated Kotlin version and added test dependencies |
| logback.xml, application.yaml | New configuration files for logging and application settings |
| FormatUtil.kt | New utility for format detection |
| IngestionResultUtils.kt | New utility for result status checking |
| OffsetDateTimeSerializer.kt | New custom serializer for date-time handling |
| mapping/*.kt | New data models for ingestion mapping configurations |
| ContainerBase.kt | New interface for container abstraction |
| LocalSource.kt, IngestionSource.kt | Removed unused source classes |
| ConfigurationApiWrapper.kt | Removed and replaced by ConfigurationClient |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (blobUrl != null) { | ||
| // Blob-based streaming | ||
| val requestBody = StreamFromBlobRequestBody(sourceUri = blobUrl) | ||
| bodyContent = Json.encodeToString(requestBody).toByteArray() |
Copilot
AI
Oct 31, 2025
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.
Missing serializer specification for Json.encodeToString(). The method requires an explicit serializer parameter: Json.encodeToString(StreamFromBlobRequestBody.serializer(), requestBody) to properly serialize the StreamFromBlobRequestBody data class.
| bodyContent = Json.encodeToString(requestBody).toByteArray() | |
| bodyContent = Json.encodeToString(StreamFromBlobRequestBody.serializer(), requestBody).toByteArray() |
| @AfterAll | ||
| fun dropTables() { | ||
| val dropTableScript = ".drop table $targetTable ifexists" | ||
| logger.error("Dropping table $targetTable") |
Copilot
AI
Oct 31, 2025
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.
Using logger.error() for normal cleanup operations is incorrect. This should be logger.info() since dropping tables in test teardown is expected behavior, not an error condition.
| logger.error("Dropping table $targetTable") | |
| logger.info("Dropping table $targetTable") |
| required: true | ||
| schema: | ||
| type: string | ||
| example: "gzip" |
Copilot
AI
Oct 31, 2025
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.
The example value for the 'Host' header parameter is set to 'gzip', which is incorrect. The Host header should contain a hostname (e.g., 'example.kusto.windows.net'), not an encoding type.
| example: "gzip" | |
| example: "ingest-mycluster.swedencentral.kusto.windows.net" |
add ingestion source blobtype to QueuedIngest