Workload feature band fallback with rollback files #24545
Conversation
There was a problem hiding this comment.
LGTM.
We also need to handle uninstalling the old versions of manifests (especially when we downgrade). I've filed #24628 for that.
Also, I believe we are now in M2 approval mode, so don't merge this yet until we get signoff.
| // TODO: figure out how to differentiate between the feature band an advertising manifest is branded as and the feature band of the SDK | ||
| // it's advertised to |
There was a problem hiding this comment.
Normally we avoid leaving TODO comments in the code when we merge. If there is follow-up work to do, it's better to track it in an issue than a code comment.
If there are no other changes, I'm OK with leaving this in so we don't have to reset the CI checks. I've made a note about this in #23403.
| Dictionary<WorkloadId, WorkloadDefinition> workloads) in manifestsToUpdate) | ||
| { | ||
| foreach ((WorkloadId WorkloadId, WorkloadDefinition workloadDefinition) in | ||
| workloads) | ||
| { | ||
| if (installedWorkloads.Contains(new WorkloadId(WorkloadId.ToString()))) | ||
| { | ||
| // TODO: Potentially show existing and new feature bands |
There was a problem hiding this comment.
Another TODO which should ideally be removed.
| packInstaller.InstalledManifests[0].manifestId.Should().Be(manifestsToUpdate[0].manifestId); | ||
| packInstaller.InstalledManifests[0].manifestVersion.Should().Be(manifestsToUpdate[0].newVersion); | ||
| packInstaller.InstalledManifests[0].sdkFeatureBand.Should().Be(new SdkFeatureBand(newSdkFeatureBand)); | ||
| packInstaller.InstalledManifests[0].offlineCache.Should().Be(null); |
There was a problem hiding this comment.
This should ideally also check the existingVersion and existingFeatureBand.
| packInstaller.InstalledManifests[0].sdkFeatureBand.Should().Be(new SdkFeatureBand("6.0.100")); | ||
| packInstaller.InstalledManifests[1].sdkFeatureBand.Should().Be(new SdkFeatureBand("6.0.300")); | ||
| packInstaller.InstalledManifests[2].sdkFeatureBand.Should().Be(new SdkFeatureBand("6.0.100")); |
There was a problem hiding this comment.
It would be a good idea to also check the existingFeatureBand for each manifest update.
fixes #23402