-
Notifications
You must be signed in to change notification settings - Fork 190
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 externally supplied worker extensions project #2763
base: main
Are you sure you want to change the base?
Conversation
Can this be reviewed/merged ? Blocks .net 9 upgrades |
@pregress how does this block .net9 upgrades for you? |
Can't upgrade what doesn't compile. If you use the
When you use version
|
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.
lgtm, some minor comments/questions. I did not look through the .targets
file changes yet
@@ -24,12 +25,20 @@ public class EnhanceExtensionsMetadata : Task | |||
[Required] | |||
public string? OutputPath { get; set; } | |||
|
|||
[Required] |
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.
Is this a breaking change to introduce a required field to a public class?
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.
I am not sure I consider this public API. It is a supporting task for our build targets, not expected to be used directly by customers.
@@ -33,20 +33,19 @@ public void GetCsProjContent_Succeeds(FuncVersion version) | |||
[InlineData(FuncVersion.V4)] |
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.
Are there any new tests that can be written to validate this behaviour?
Issue describing the changes in this PR
resolves #2601
resolves #1252
resolves #1888
resolves #2125
Pull request checklist
release_notes.md
-- TODOAdditional information
This PR refactors the SDK to address multiple issues while introducing the ability to have the worker extension project externally supplied.
FunctionsAssignTargetPaths
was removed as we can no longer predict the paths that early in the build. Instead we calculate them all later on. This also addresses Regression inMicrosoft.Azure.Functions.Worker.Sdk
version1.17.4
/1.17.3
of solution builds using no-incremental #2601.<FunctionsGenerateExtensionProject>false</FunctionsGenerateExtensionProject>
and<ExtensionsCsProj>{{PATH OF EXTERNAL PROJECT}}</ExtensionsCsProj>
dotnet build -t:GenerateExtensionProject
on the function project. This will one-time generate the extension project to theExtensionsCsProj
path. It is then assumed that the generated project is checked into source control and included in solution files / build projects as necessary.dotnet build -t:GenerateExtensionProject
again.GenerateExtensionProject
target as necessary or to manually update webjobs extensions.