Migrate to multi-version connector packaging (Option B)#238
Open
andrew-hardy wants to merge 12 commits into
Open
Migrate to multi-version connector packaging (Option B)#238andrew-hardy wants to merge 12 commits into
andrew-hardy wants to merge 12 commits into
Conversation
Targets 4.6.0, 4.7.1, 4.8.0 (net6.0, matching each release/4.x branch's own Packages.props _CluedIn value) and 5.0.0-* (net10.0, this repo's own current default). All three older release lines are already fully merged into develop (confirmed via git merge-base --is-ancestor). CluedInMultiVersionTargetFramework added to Directory.Build.props for the same reason as the other multi-version repos - the older targets need net6.0, which requires overriding TargetFramework across this repo's Connector.FileStorage.Common -> Connector.DataLake.Common -> per-connector ProjectReference chain. Test tooling (Microsoft.NET.Test.Sdk, xunit, AutoFixture) is now conditioned on the same >= 5.0.0 CluedIn-line boundary, referencing the exact xunit v2 / AutoFixture.Xunit2 / Test.Sdk 16.5.0 versions the 4.6/4.7/4.8 release lines already used for net6.0 (confirmed via git show against each release branch) - the newer xunit.v3/Microsoft Testing Platform-based tooling this repo defaults to does not support net6.0. Centralized into test/Directory.Build.props's previously-empty ItemGroup instead of repeating the same package list across all three test projects. Retains executeIntegrationTests via crawler.build.jobs.yml's new support for it (see AzurePipelines.Templates), and the existing AzureDataLakeConnector_BuildLock exclusive lock/lockBehavior: sequential unchanged.
…hape IStreamRepository.GetStream/GetAllStreams/GetOrganizationStreams/ GetOrganizationStreamsCount all take an ExecutionContext (and are async, for GetAllStreams) from CluedIn.Core 4.7 onward - in 4.6.0 they take a plain Guid instead (or no parameter, synchronously, for GetAllStreams). Confirmed via reflection against the actual restored 4.6.0 packages (System.Reflection.MetadataLoadContext), not guessed. Affects six call sites across Connector.FileStorage.Common and Connector.AzureDataLake.
GetStreamMappings(Guid) takes no ExecutionContext, and SetupConnector's ExecutionContext parameter is last (streamId, model, context), not first, in CluedIn.Core 4.6.0 - confirmed via reflection against the actual restored 4.6.0 package.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the connector repo to a single-branch, multi-version packaging model where CI builds multiple CluedIn-target variants in parallel and bundles them into one NuGet package, while handling CluedIn.Core API differences via conditional compilation.
Changes:
- Centralizes/conditions test dependencies (xUnit v2 vs v3) to support both net6.0 (CluedIn 4.x) and net10.0 (CluedIn 5.x).
- Adds
#if CLUEDIN_V47call-site guards forIStreamRepositoryAPI shape differences across CluedIn versions. - Switches the pipeline to the
crawler.build.jobs.ymltemplate with multi-version target configuration and an integration-test job hook.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/Connector.FileStorage.Common.Tests.Unit/Connector.FileStorage.Common.Tests.Unit.csproj | Removes per-project test package refs to rely on shared test props. |
| test/unit/Connector.AzureDataLake.Tests.Unit/Connector.AzureDataLake.Tests.Unit.csproj | Removes per-project test package refs to rely on shared test props. |
| test/unit/Connector.AmazonS3.Tests.Unit/Connector.AmazonS3.Tests.Unit.csproj | Removes per-project test package refs to rely on shared test props. |
| test/Directory.Build.props | Adds shared test package references and conditions for xUnit v2/v3 by target. |
| src/Connector.FileStorage.Common/StreamsHelper.cs | Adds version-conditional GetAllStreams call shape. |
| src/Connector.FileStorage.Common/EventHandlers/UpdateStreamScheduleBase.cs | Adds version-conditional GetStream call shape. |
| src/Connector.FileStorage.Common/EventHandlers/UpdateExportTargetEventHandler.cs | Adds version-conditional organization stream query call shapes. |
| src/Connector.FileStorage.Common/Connector/StorageExportEntitiesJobBase.cs | Adds version-conditional GetStream call shapes in multiple methods. |
| src/Connector.FileStorage.Common/Connector/StorageConnector.ConnectorActions.cs | Adds version-conditional GetStream call shape. |
| src/Connector.AzureDataLake/AzureDataLakeDataMigrator.cs | Adds version-conditional stream/mapping/setup calls for 4.6 vs 4.7+. |
| Packages.props | Introduces CluedIn version parsing, defines CLUEDIN_Vxx constants, and conditions test dependency version pins by CluedIn line. |
| Directory.Build.props | Supports CI-driven multi-version target framework selection via CluedInMultiVersionTargetFramework. |
| azure-pipelines.yml | Migrates to multi-version jobs template, adds probe parameter, and configures version/TFM matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
IStreamRepository is mocked in integration tests, not backed by a real CluedIn host, and the mock was hardcoded to the 4.7+ GetStream(ExecutionContext, Guid) overload. That overload doesn't exist on CluedIn.Core 4.6.0 - only GetStream(Guid) does - so this couldn't compile against, let alone correctly exercise, the pre-4.7 code path once integration tests start running per target instead of once against the repo's own default. Mirrors the same guard already in UpdateStreamScheduleBase.cs.
Every test/integration/*.csproj hardcoded AutoFixture.Xunit3/xunit.v3/ Microsoft.NET.Test.Sdk/coverlet.msbuild/xunit.runner.visualstudio/Moq unconditionally, duplicating (and, for AutoFixture.Xunit3/xunit.v3, conflicting with) test/Directory.Build.props' own CluedIn-version-gated selection of the same packages - already relied on by test/unit projects after an earlier cleanup, never extended to test/integration. This only surfaced now that integration tests build against every multi-version target instead of only the repo's own default: Packages.props only defines a version for AutoFixture.Xunit3/xunit.v3 for CLUEDIN_V50, so an unconditional reference to them fails to restore below that line (NU1010 'must have a version defined').
ITestOutputHelper lives directly in the Xunit namespace under xunit.v3 (CLUEDIN_V50, this repo's own default), but under the separate Xunit.Abstractions namespace/package for the xunit v2 tooling the 4.6/4.7/4.8 lines use. A plain 'using Xunit;' only resolves it under v3, so building any integration test project against an older target failed with CS0246 - the first time this gap surfaced, since no test/unit file uses ITestOutputHelper at all.
IAsyncLifetime.InitializeAsync/DisposeAsync return ValueTask under xunit.v3 (CLUEDIN_V50, this repo's own default) but Task under the xunit v2 tooling the 4.6/4.7/4.8 lines use - a breaking interface change, not just a version-gated package choice. This is the only integration test file implementing IAsyncLifetime, and the only one that failed once integration tests started building against every target instead of only the default: FabricOpenMirroring, OneLake, DataLake.Common and AzureDataLake's own integration tests all already ran and passed for real against CluedIn 4.6.0 before hitting this.
VerifyStoreData_Sync_WithStreamCacheCanUseTableName and Archive_Sync_CanDeleteDirectoryWhenUseTableName hardcoded TableName to 'MyTable'/'ToBeArchived'. RootDirectoryPath is derived from the shared MirroredDatabaseName env var, so these resolved to the same real ADLS directory across every concurrent CI job - and Archive_Sync actively deletes that directory, meaning one job's run could delete a directory another concurrent job's test was still using. Suffixing both with a GUID makes each test run's directory independent, a prerequisite for running multi-version integration tests in true parallel instead of sequentially.
…erent versions of cluedin into one line # Conflicts: # Packages.props a
…ion-packaging # Conflicts: # Directory.Build.props # Packages.props # src/Connector.AzureDataLake/AzureDataLakeDataMigrator.cs # src/Connector.FileStorage.Common/Connector.FileStorage.Common.csproj # src/Connector.FileStorage.Common/EventHandlers/UpdateExportTargetEventHandler.cs # src/Connector.FileStorage.Common/StreamRepositoryExtensions.v47_to_Latest.cs # test/Directory.Build.props # test/integration/Connector.AmazonS3.Tests.Integration/AmazonS3WriteStreamTests.cs # test/integration/Connector.AzureDataLake.Tests.Integration/Connector.AzureDataLake.Tests.Integration.csproj # test/integration/Connector.DataLake.Common.Tests.Integration/Connector.DataLake.Common.Tests.Integration.csproj # test/integration/Connector.FabricOpenMirroring.Tests.Integration/Connector.FabricOpenMirroring.Tests.Integration.csproj # test/integration/Connector.FabricOpenMirroring.Tests.Integration/FabricOpenMirroringConnectorTests.cs # test/integration/Connector.OneLake.Tests.Integration/Connector.OneLake.Tests.Integration.csproj
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
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.
Description
Moves this repo off its three separate release branches (`release/4.6.6`, `release/4.7.4`, `release/4.8.1`, all confirmed already merged into `develop`) onto CluedIn-io/AzurePipelines.Templates' `crawler.build.jobs.yml` (Option B): a single branch, with CI building one variant per supported CluedIn version in parallel and bundling them into one NuGet package with a build-time switch.
Targets 4.6.0, 4.7.1, 4.8.0 and 5.0.0-* (derived from each former release branch's own `Packages.props`). This is the most complex repo migrated onto the new scheme so far: 9 packable projects under `src/` (AmazonS3, AzureDataLake, DataLake.Common, FabricOpenMirroring, FileStorage.Common, OneLake, SynapseDataEngineering, AzureDatabricks, AzureAIStudio), each independently discovered and multi-version-packaged.
`IStreamRepository`'s API on CluedIn.Core changed shape between 4.6.0 and 4.7+ (parameter order, sync→async, `ExecutionContext` added/reordered) - isolated with `#if CLUEDIN_V47` guards at every affected call site, verified against real reflection of the historical CluedIn.Core assemblies rather than guessed.
Also extends the shared template itself with a new dedicated integration-test job (`executeIntegrationTests`), since this repo's existing integration test suite needed a home in the new pipeline shape.
Work Item ID: AB#57964
How has it been tested?
CI builds, unit-tests, packs and verifies cleanly against all four targets, and the new integration-test job runs the full existing integration suite end-to-end.
Release Note
Connector builds for CluedIn 4.6-5.0 now ship from a single published package instead of separately versioned per-line packages.