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

HostFactoryResolver - Increase default timeout and add env var option #61688

Merged
merged 4 commits into from Nov 17, 2021

Conversation

maryamariyan
Copy link
Member

@maryamariyan maryamariyan commented Nov 16, 2021

Customer Impact

When customer has debugger attached or when using tools like EF migration, host startup path using WebApplicationBuilder for hosting and WebApplicationFactory<T> for testing, may fail to build and throw an InvalidOperationException with the message “Unable to build IHost.”, because the default timeout is only 5 seconds.

We are proposing to use a higher timeout (5 minute) to be used so that when an application is taking longer to build, we could allow startup code to complete, and the host can start successfully. Check comment here for more detail.

Increasing the timeout should not have a negative impact on experience.

We also added an environment variable option called DOTNET_HOST_FACTORY_RESOLVER_DEFAULT_TIMEOUT_IN_SECONDS in case user needs to set their own custom timeout.

See issue: #60891

Testing

Check PR #61621

Risk

Very low.

Regression

No, new 6.0 feature/code flow for discovering a host

cherry pick of #61621

@msftbot
Copy link
Contributor

msftbot bot commented Nov 16, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: maryamariyan
Assignees: -
Labels:

area-Extensions-Hosting

Milestone: -

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

We need packaging changes. See: 39ecea6#diff-c7518d66e82e24aa3bb3bcf33bfa8717242e02d446983237f6ae56602785fde6R12 for an example.

Make sure that the package is created locally when building all configurations leg.

@eerhardt
Copy link
Member

We need packaging changes.

I'm not sure if it matters, but this package isn't a traditional library package.

  1. It is a "source-only" package
  2. It isn't shipping on nuget.org. It is only created for other teams (like EF) to consume.

Hopefully that doesn't change how we service this package, but I just wanted to call that out.

@safern
Copy link
Member

safern commented Nov 16, 2021

Thanks, @eerhardt for calling that out.

I'm not sure how we build source packages, but I believe we still need the GeneratePackageOnBuild set to true in order for the servicing builds to generate the package. Just looked at the latest release/6.0 official build and it didn't produce this package at all.

I would still bump the servicing version even though it doesn't land on NuGet.org as that helps reason about versions even for internal consumption, i.e aspnet transport packages also get that patch version bumped.

@ericstj
Copy link
Member

ericstj commented Nov 16, 2021

I would still bump the servicing version

Looks like this happens automatically for this package.

@ericstj
Copy link
Member

ericstj commented Nov 16, 2021

Looks like this happens automatically for this package.

Or perhaps that's the default unless you specify ServicingVersion. In which case we should specify it. Aside: I wonder if we can enforce this in servicing so that folks don't accidentally miss it.

…opLevelStatementsTestsTimeout/TopLevelStatementsTestsTimeout.csproj
@safern
Copy link
Member

safern commented Nov 17, 2021

Tests are definitely unrelated. I will open issues for them.

@safern safern merged commit c91170a into dotnet:release/6.0 Nov 17, 2021
ML, Extensions, Globalization, etc, POD. automation moved this from Active PRs to Done Nov 17, 2021
@msftbot msftbot bot locked as resolved and limited conversation to collaborators Dec 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants