[Projects] Simplify shared props & split Microsoft.Azure.EventHubs.Processor dependency #311
Conversation
This file contains 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
…ocessor dependency Overall changes: - Utilized Directory.Build.props to DRY up projects (and misc. associated changes like one icon.png file, etc.) - Creates a new DurableTask.Netherite.EventHubs project which is the only thing depending on Microsoft.Azure.EventHubs.Processor. - Moves all EventHubs code possible to DurableTask.Netherite.EventHubs The problem here is there are still a few refs in core classes we have to figure out: - `ScalingMonitor` is calling `EventHubsConnections.GetQueuePositionsAsync` to see if work is left - `FasterStorageProvider` is calling `EventHubsUtil.DeleteEventHubIfExistsAsync` to cleanup partition hubs to ensure queues start at zero Other than those 2, the split is pretty clean here. I do recommend we fully drop `netcoreapp2.2` though - I assume based on comments and other cleanup that has happened it's explicitly here for a reason I don't understand, happy to understand that and maybe advise on some alternatives to clean up the build if we still need to keep it around.
|
I think we now have addressed this. The most urgent problem (scale controller conformance) has been solved by updating the scale controller selectively, I believe. As for the rest of the codebase, we now have a PR that updates all uses of the SDK here: #385 |
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.
Draft PR for discussion of #310
I figured this was the easiest way to envision what a split looks like, so starting a PR to help discussion on what ties remain problematic.
This is the hopefully happy path proposal of splitting the dependency to defer the upgrade.
Overall changes:
The problem here is there are still a few refs in core classes we have to figure out:
ScalingMonitoris callingEventHubsConnections.GetQueuePositionsAsyncto see if work is leftFasterStorageProvideris callingEventHubsUtil.DeleteEventHubIfExistsAsyncto cleanup partition hubs to ensure queues start at zeroOther than those 2 usages, the split is pretty clean here. I do recommend we fully drop
netcoreapp2.2though - I assume based on comments and other cleanup that has happened it's explicitly here for a reason I don't understand, happy to understand that and maybe advise on some alternatives to clean up the build if we still need to keep it around.