Skip to content
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

Add support for setting an additional assembly scanning path #5653

Merged
merged 7 commits into from
Jun 16, 2020

Conversation

boblangley
Copy link
Member

Partially addresses https://github.com/Particular/NServiceBus.AzureFunctions/issues/44 part 1: The Core doesn't allow assembly scanning with an alternate path

@bording
Copy link
Member

bording commented Jun 2, 2020

I don't think this new setting should override the existing scan locations, but should be an additional scan location that can be configured.

@SeanFeldman SeanFeldman force-pushed the custom-assembly-scanning-paths branch from ef62338 to c9f51b5 Compare June 2, 2020 22:08
@SeanFeldman
Copy link
Contributor

I don't think this new setting should override the existing scan locations, but should be an additional scan location that can be configured.

Could you please provide an example when an additional location would be used in conjunction with the predefined locations, @bording?

@bording
Copy link
Member

bording commented Jun 4, 2020

The thought process was that if we are exposing this API publicly, then we should make it generally safe to use. If we replace the default location when this is called, then assembly scanning breaks if all of the assemblies that need to be scanned aren't in that location (minus whatever happens to already be loaded if appdomain scanning is enabled)

If this is an additional location that can be scanned, then someone could locate assemblies in some other path and have those picked up in addition to the assemblies in the default locations.

@timbussmann
Copy link
Contributor

If this is an additional location that can be scanned, then someone could locate assemblies in some other path and have those picked up in addition to the assemblies in the default locations.

I think this could be done by the user by just loading whatever they want from wherever into the AppDomain (or load contexts) manually and NServiceBus should pick it up as part of scanning the loaded assemblies. That logic should be easy to implement given that in such cases there isn't a lot of "filtering" and optimizations required. At least from this perspective, this can already be achieved today if there is an urgent need for it and it would not depend on the implementation of this feature.

@timbussmann
Copy link
Contributor

btw. I think there is an interesting angle to this to "disable" the assembly scanning by providing a different path that wouldn't be possible when using "additional" scanning locations. Arguably that might be more of a "misuse" of a feature but I at least that would be a feature more appealing to me :D

@SeanFeldman SeanFeldman force-pushed the custom-assembly-scanning-paths branch from c9f51b5 to 11221bd Compare June 9, 2020 20:58
@SeanFeldman SeanFeldman changed the title Add support for setting a custom assembly scanning path Add support for setting an additional assembly scanning path Jun 9, 2020
@SeanFeldman
Copy link
Contributor

The PR was updated to support an additional path rather than replacing a path.

Co-authored-by: Bob Langley <bob.langley@particular.net>
@SeanFeldman SeanFeldman force-pushed the custom-assembly-scanning-paths branch from 0b8321c to e8874d3 Compare June 9, 2020 21:28
timbussmann
timbussmann previously approved these changes Jun 10, 2020
{
if (TryLoadScannableAssembly(assemblyFile.FullName, results, out var assembly))
foreach (var assemblyFile in ScanDirectoryForAssemblyFiles(directoryToScan, ScanNestedDirectories))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that nested directories for the additional scanning path will also be potentially scanned. I'm wondering if this should be an option for the additional scanning path.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know but assuming the assumptions about the primary path should apply to the additional path.

src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs Outdated Show resolved Hide resolved
@SeanFeldman SeanFeldman requested a review from bording June 16, 2020 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants