Skip to content

Conversation

RichardChen820
Copy link
Contributor

Shorten the default timeout of individual call to backend

@zhenlan zhenlan requested a review from Copilot February 17, 2025 22:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs:1136

  • Ensure that the new timeout behavior introduced by checking for TaskCanceledException with an inner TimeoutException is covered by tests.
TaskCanceledException tce = ex.InnerExceptions?.LastOrDefault(e => e is TaskCanceledException) as TaskCanceledException;

@RichardChen820 RichardChen820 force-pushed the user/junbchen/shorten_timeout_of_each_call branch from 71a346e to 4e7f8b0 Compare February 24, 2025 02:06
@zhenlan
Copy link
Contributor

zhenlan commented Feb 26, 2025

@RichardChen820 why don't merge this?

@amerjusupovic @jimmyca15 is there a plan to release 8.1.1? Should this be included?

@zhenlan
Copy link
Contributor

zhenlan commented Feb 27, 2025

Discussed offline. Won't block the 8.1.1 train. This change will be included in the next release.

@amerjusupovic amerjusupovic force-pushed the user/junbchen/shorten_timeout_of_each_call branch 5 times, most recently from 2dd4ed8 to b89c6d0 Compare March 12, 2025 21:54
@amerjusupovic amerjusupovic force-pushed the user/junbchen/shorten_timeout_of_each_call branch from b89c6d0 to 8030c60 Compare March 12, 2025 21:57
@amerjusupovic amerjusupovic merged commit 87f0f85 into main Mar 18, 2025
3 checks passed
@amerjusupovic amerjusupovic deleted the user/junbchen/shorten_timeout_of_each_call branch March 18, 2025 19:49
@RichardChen820
Copy link
Contributor Author

RichardChen820 commented Mar 19, 2025 via email

@jimmyca15
Copy link
Member

Why did this PR go in with every single line of AzureAppConfigurationOptions changed?

@amerjusupovic
Copy link
Contributor

Why did this PR go in with every single line of AzureAppConfigurationOptions changed?

It's my fault, I couldn't find a way to fix it, but I should have just redone the PR. I'll revert this and put up a new one.

amerjusupovic added a commit that referenced this pull request Apr 21, 2025
@jimmyca15
Copy link
Member

@amerjusupovic Right, we shouldn't merge the PR if it has something malformatted like that, but fixing it would be on the author's plate.

amerjusupovic added a commit that referenced this pull request Apr 21, 2025
amerjusupovic added a commit that referenced this pull request Apr 21, 2025
@RichardChen820
Copy link
Contributor Author

RichardChen820 commented Apr 22, 2025

@amerjusupovic I guess the unexpected change comes from rebase - conflict resolution - force commit? I ensure I didn't have those changes in my PR.

@amerjusupovic
Copy link
Contributor

@amerjusupovic I guess the unexpected change comes from rebase - conflict resolution - force commit? I ensure I didn't have those changes in my PR.

Yes @RichardChen820 these issues were all just as a result of inconsistent line endings in the repo and conflict resolution when I tried to push them, sorry about that. I'm resending your changes with just a slight change here based on feedback from @jimmyca15: #657

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.

6 participants