Skip to content

Add support for default_headers#270

Merged
mkistler merged 2 commits intomasterfrom
mdk/default-headers
Oct 5, 2017
Merged

Add support for default_headers#270
mkistler merged 2 commits intomasterfrom
mdk/default-headers

Conversation

@mkistler
Copy link
Copy Markdown

@mkistler mkistler commented Oct 3, 2017

This PR adds support for a set_default_headers in the base WatsonDeveloperCloudService class. This is a more general and flexible solution for setting the x-watson-learning-opt-out header. I left the x_watson_learning_opt_outparameter on the service constructor for backward compatibility -- we should consider removing that on the next major release.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 3, 2017

Codecov Report

Merging #270 into master will decrease coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
- Coverage   85.23%   85.22%   -0.01%     
==========================================
  Files          24       24              
  Lines        1219     1225       +6     
==========================================
+ Hits         1039     1044       +5     
- Misses        180      181       +1
Impacted Files Coverage Δ
..._developer_cloud/watson_developer_cloud_service.py 76.26% <88.88%> (+0.22%) ⬆️

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 c12c9aa...323eef7. Read the comment docs.

Copy link
Copy Markdown
Contributor

@glennrfisher glennrfisher left a comment

Choose a reason for hiding this comment

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

Looks good!

Comment thread test/test_alchemy_language_v1.py Outdated
inited = watson_developer_cloud.AlchemyLanguageV1(url=default_url, api_key='boguskey',
x_watson_learning_opt_out=True)
assert inited.x_watson_learning_opt_out
#assert inited.x_watson_learning_opt_out
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can delete this line, since it's no longer needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

assert message is not None
assert responses.calls[0].request.url == message_url1
assert 'x-watson-learning-opt-out' in responses.calls[0].request.headers
assert responses.calls[0].request.headers['x-watson-learning-opt-out'] == 'true'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice check on the response. More confidence-inspiring than the previous assert inited.x_watson_learning_opt_out.

@germanattanasio germanattanasio self-requested a review October 4, 2017 22:06
@mkistler mkistler merged commit f4d3e36 into master Oct 5, 2017
@mkistler mkistler deleted the mdk/default-headers branch October 5, 2017 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants