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

Updating FunctionsNetHost (dotnet isolated worker) to 1.0.9 #10262

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

kshyju
Copy link
Member

@kshyju kshyju commented Jul 1, 2024

Updating FunctionsNetHost (dotnet isolated worker) to 1.0.9. Azure/azure-functions-dotnet-worker#2552
Updated a test(which uses the actual worker configs) to reflect this change.

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

  • Backporting to the in-proc branch is not required
  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

@kshyju kshyju marked this pull request as ready for review July 2, 2024 00:00
@kshyju kshyju requested a review from a team as a code owner July 2, 2024 00:00
@kshyju
Copy link
Member Author

kshyju commented Jul 2, 2024

Many tests depend on the worker config provided by the referenced worker packages. The new DNI worker config requires a specific environment variable (WEBSITE_PLACEHOLDER_MODE) to be present. If this variable is absent, the worker will be skipped. Therefore, the tests need to include this environment variable. I am currently working on fixing them.

@@ -1600,7 +1600,6 @@ public async Task Initialize_LogsWarningForExplicitlySetHostId()

[Theory]
[InlineData("python", "main.py", "python", "python")]
[InlineData("dotnet-isolated", "app.dll", "dotnet-isolated", "dotnet-isolated")]
Copy link
Member Author

Choose a reason for hiding this comment

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

The dotnet isolated worker config was updated to load only in placeholder mode. Few other tests, I was able to set the environment variable and that allows loading the dotnet isolated worker config. But for this test, there are other code path for which this test is running, where we return if we are in placeholder mode.

Considering the fact that, we are in the process of making this app setting a mandatory value, I think it may be okay to remove the "dotnet-isolated" case from this test.

@kshyju kshyju requested review from jviau and fabiocav July 3, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants