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

Correctly handle HTTP errors in RequestLogPolicy #217

Merged
merged 1 commit into from
Nov 4, 2020

Conversation

pharaujo
Copy link
Contributor

The condition that tests the HTTP status code to decide whether or not to upgrade log severity never sees the 4xx and 5xx responses because they are encapsulated in a StorageError.

Addresses #214

@ghost
Copy link

ghost commented Oct 19, 2020

CLA assistant check
All CLA requirements met.

The condition that tests the HTTP status code to decide whether or not to upgrade log severity never sees the 4xx and 5xx responses because they are encapsulated in a `StorageError`.

Addresses Azure#214
@pharaujo
Copy link
Contributor Author

pharaujo commented Nov 3, 2020

Hey @mohsha-msft the build failure doesn't seem related to my change, as both master and dev branches seem to be crashing in the same way when I run the test locally.

How should I proceed?

@mohsha-msft
Copy link
Contributor

@pharaujo

We appreciate your efforts to improve Azure Blob Go SDK!

The build pipeline uses private credentials (storage account name and account key) which we do not expose to public PRs. Please raise the review request anyway. We'll take a look at the PR and will run the tests after that using our credentials.

For more clarification, reach out to us.

@pharaujo
Copy link
Contributor Author

pharaujo commented Nov 3, 2020

@mohsha-msft

(...) Please raise the review request anyway. We'll take a look at the PR and will run the tests after that using our credentials.

This already is the PR with the logging fix. Do you need anything else from my end?

@mohsha-msft mohsha-msft changed the base branch from master to dev November 3, 2020 14:52
@mohsha-msft mohsha-msft changed the base branch from dev to issue/http-log-policy November 4, 2020 15:46
@mohsha-msft mohsha-msft merged commit a3810da into Azure:issue/http-log-policy Nov 4, 2020
@mohsha-msft
Copy link
Contributor

mohsha-msft commented Nov 4, 2020

Merging the branch with issue/http-log-policy branch to run regression tests. Raised a separate PR to merge issue/http-log-policy with dev #228

@mohsha-msft mohsha-msft mentioned this pull request Nov 4, 2020
@pharaujo pharaujo deleted the patch-1 branch November 4, 2020 15:58
@pharaujo pharaujo restored the patch-1 branch November 4, 2020 16:36
@pharaujo pharaujo deleted the patch-1 branch November 4, 2020 16:47
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.

2 participants