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.

Sign up for GitHub

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

Jump to bottom

Don't Log First Exception when Connecting to Proxy #30274

Merged
merged 12 commits into from
Dec 2, 2022
Merged

Don't Log First Exception when Connecting to Proxy #30274

merged 12 commits into from
Dec 2, 2022

Conversation

alzimmermsft
Copy link
Member

Description

Fixes #30148

Updates Netty proxy handling to not throw an exception on the first attempt to connect to the proxy. Credentials aren't applied on the first request so it is common for this to fail.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@ghost ghost added the Azure.Core azure-core label Aug 4, 2022
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@alzimmermsft alzimmermsft marked this pull request as ready for review August 4, 2022 15:33
lmolkova
lmolkova previously approved these changes Aug 30, 2022
View reviewed changes
@alzimmermsft
Copy link
Member Author

/azp run java - core - tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@alzimmermsft
Copy link
Member Author

/azp run java - core

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alzimmermsft alzimmermsft self-assigned this Aug 31, 2022
@alzimmermsft
Copy link
Member Author

Running live test for Storage is showing OutOfMemoryErrors which need to be investigated further before merging

@alzimmermsft alzimmermsft dismissed lmolkova’s stale review August 31, 2022 18:40

Need a full review was OOME is fixed

@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Nov 4, 2022
@ghost
Copy link

ghost commented Nov 4, 2022

Hi @alzimmermsft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@nandorsoma
Copy link

Hey! We still need this fix. It would be nice if it wasn't lost.

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Nov 9, 2022
@alzimmermsft
Copy link
Member Author

Hey! We still need this fix. It would be nice if it wasn't lost.

Wasn't lost! Just been bogged down with a few other tasks. I'm getting back around to this now 😄

@alzimmermsft alzimmermsft merged commit 1c0fa83 into Azure:main Dec 2, 2022
@alzimmermsft alzimmermsft deleted the AzNetty_DontLogExceptionOnFirstProxyConnectAttempt branch December 2, 2022 21:10
@nandorsoma
Copy link

Hey! We still need this fix. It would be nice if it wasn't lost.

Wasn't lost! Just been bogged down with a few other tasks. I'm getting back around to this now 😄

No problem, I just commented to remove the flag. Thanks for pulling it off!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmolkova lmolkova lmolkova approved these changes

@srnagar srnagar Awaiting requested review from srnagar srnagar is a code owner

@anuchandy anuchandy Awaiting requested review from anuchandy anuchandy is a code owner

@vcolin7 vcolin7 Awaiting requested review from vcolin7 vcolin7 is a code owner

@mssfang mssfang Awaiting requested review from mssfang mssfang is a code owner

@JonathanGiles JonathanGiles Awaiting requested review from JonathanGiles JonathanGiles is a code owner

@billwert billwert Awaiting requested review from billwert billwert is a code owner

@kasobol-msft kasobol-msft Awaiting requested review from kasobol-msft

Assignees

@alzimmermsft alzimmermsft

Labels
Azure.Core azure-core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] 407 Proxy Authentication Required should not trigger warning log
4 participants
@alzimmermsft @azure-sdk @nandorsoma @lmolkova