Skip to content

Conversation

@lmazuel
Copy link
Member

@lmazuel lmazuel commented Feb 15, 2018

@derekbekoe @johanste
This disables HTTP log by default and add two ways to configure it:

  • At the operation level: create_or_update(....., enable_http_log=True)
  • At the client level: client.config.enable_http_log=True
    Operation level is most important (i.e. if operation says False and client says True, we don't log)

I would recommend the CLI to add client.config.enable_http_log=True in the generic client factory. And to override it by False for known critical operation if necessary (KeyVault? etc.)

I don't merge until you tell me the CLI is ready.

@codecov-io
Copy link

codecov-io commented Feb 16, 2018

Codecov Report

Merging #86 into master will decrease coverage by 0.19%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #86     +/-   ##
=========================================
- Coverage   78.05%   77.85%   -0.2%     
=========================================
  Files         273      273             
  Lines       14298    14300      +2     
=========================================
- Hits        11160    11133     -27     
- Misses       3138     3167     +29
Impacted Files Coverage Δ
msrest/configuration.py 44% <100%> (+0.75%) ⬆️
msrest/service_client.py 82.18% <60%> (-1.06%) ⬇️
msrest/http_logger.py 13.33% <0%> (-60%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b79de42...a846fbc. Read the comment docs.

@lmazuel lmazuel merged commit 5df4c93 into master Mar 7, 2018
@lmazuel lmazuel deleted the logoption branch March 7, 2018 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants