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

Logging policy #7988

Closed
wants to merge 5 commits into from
Closed

Logging policy #7988

wants to merge 5 commits into from

Conversation

lmazuel
Copy link
Member

@lmazuel lmazuel commented Oct 17, 2019

Fix #7987

@lmazuel lmazuel added Client This issue points to a problem in the data-plane of the library. Azure.Core blocking-release Blocks release labels Oct 17, 2019
@AutorestCI
Copy link
Contributor

AutorestCI commented Oct 17, 2019 •
edited

(message created by the CI based on PR content)

Installation instruction

Package azure-core

You can install the package azure-core of this PR using the following command:
pip install "git+https://github.com/Azure/azure-sdk-for-python@feature/logging_policy#egg=azure-core&subdirectory=azure-core"

You can build a wheel to distribute for test using the following command:
pip wheel --no-deps "git+https://github.com/Azure/azure-sdk-for-python@feature/logging_policy#egg=azure-core&subdirectory=azure-core"

If you have a local clone of this repository, you can also do:

  • git checkout feature/logging_policy
  • pip install -e ./azure-core

Or build a wheel file to distribute for testing:

  • git checkout feature/logging_policy
  • pip wheel --no-deps ./azure-core

Direct download

Your files can be directly downloaded here:

@lmazuel lmazuel marked this pull request as ready for review October 17, 2019 21:22
@lmazuel lmazuel requested a review from johanste October 17, 2019 21:22
@weshaggard weshaggard closed this Oct 18, 2019
@weshaggard
Copy link
Member

@lmazuel closing this temporarily to try and reset the devops pipelines.

@lmazuel lmazuel reopened this Oct 18, 2019
xiangyan99
xiangyan99 reviewed Oct 18, 2019
View reviewed changes
Copy link
Member

@xiangyan99 xiangyan99 left a comment •
edited

Choose a reason for hiding this comment

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

You may also need to check

  1. if we want to update configuration.py?
  2. do we want to add it into default pipeline?

sdk/core/azure-core/azure/core/pipeline/policies/_universal.py Show resolved Hide resolved
sdk/core/azure-core/azure/core/pipeline/policies/_universal.py Show resolved Hide resolved
@mikeharder
Copy link
Member

Closing since this PR keeps triggering all Python pipelines. Please open a new PR instead of re-opening this one.

@mikeharder mikeharder closed this Oct 18, 2019
@lmazuel lmazuel mentioned this pull request Oct 18, 2019
Closed
johanste
johanste reviewed Oct 19, 2019
View reviewed changes
sdk/core/azure-core/azure/core/pipeline/policies/_universal.py
"azure.core.pipeline.policies.http_logging_policy"
)
self.allowed_query_params = set()
self.allowed_header_namers = set(HttpLoggingPolicy.DEFAULT_HEADERS_WHITELIST)
Copy link
Member

Choose a reason for hiding this comment

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

Spelling allowed_header_namers should be allowed_header_names?

johanste
johanste reviewed Oct 19, 2019
View reviewed changes
Copy link
Member

@johanste johanste left a comment

Choose a reason for hiding this comment

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

Possible spelling mistake, otherwise rock'n roll!

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

@xiangyan99 xiangyan99 xiangyan99 left review comments

@johanste johanste johanste left review comments

@bryevdv bryevdv Awaiting requested review from bryevdv

Assignees
No one assigned
Labels
Azure.Core blocking-release Blocks release Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding logging policy
6 participants
@lmazuel @AutorestCI @weshaggard @mikeharder @xiangyan99 @johanste