Update sdkVersion in graph_session.py #46
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes microsoftgraph#42 Prepended sdkVersion with comma separated new values in self.headers in graph_session.py Modified a test and added another test to check whether the value has been updated correctly
|
|
jobala
reviewed
Apr 29, 2020
msgraphcore/graph_session.py
Outdated
Comment on lines
23
to
27
| if 'sdkVersion' not in self.headers: | ||
| self.headers.update({'sdkVersion': 'graph-python-' + SDK_VERSION}) | ||
| else: | ||
| self.headers.update({'sdkVersion': 'graph-python-' + SDK_VERSION + ', ' | ||
| + self.headers.get('sdkVersion')}) |
There was a problem hiding this comment.
Put this functionality in a private method _append_sdk_version and call the function in the constructor.
I prefer checking if sdkVersion is in headers first
if 'sdkVersion' in self.headers:
...
else:
...There was a problem hiding this comment.
Sure I'll make the changes!
|
I have come up with this definition of def _append_sdk_version(self) -> None:
"""Updates sdkVersion in headers with comma-separated new values
"""
if 'sdkVersion' in self.headers:
self.headers.update({'sdkVersion': 'graph-python-' + SDK_VERSION + ', '
+ self.headers.get('sdkVersion')})
else:
self.headers.update({'sdkVersion': 'graph-python-' + SDK_VERSION})Please let me know what the docstring should be. Is this correct? |
|
Looks good @rajatdiptabiswas |
Created a private method _append_sdk_version to update sdkVersion in headers
|
Made the changes you mentioned @jobala. Thanks in advance! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #42
Prepended
sdkVersionwith comma-separated new values inself.headersingraph_session.pyModified a test and added another test to check whether the value has been updated correctly
Kindly check if everything is correct.
If something is wrong, please let me know. I'll try to get it fixed.