Repository: slackapi/python-slackclient
Pull request #418 (2019-05-13 22:11:00)

Modify package identifier in user agent to match v1.x identifier
###  Summary

Since `v2.0.0`, the package identifier in each request (embedded in the `User-Agent` header) changed from `slackclient` to `slack`. This change was not intentional, it was incidental on the fact that the code read the identifier from the module name using the magic variable `__name__`.

That change introduces issues in how Slack measures the usage of this tool. `slack` lacks the uniqueness, since other tools including the Desktop apps report themselves as `slack`. This forces classification of a request to require knowing the entire User Agent header to look for `python` alongside `slack` to distinguish. Ideally, the parsing logic can break the User Agent string on spaces, and classify each portion separately. It also creates a discontinuity so we cannot measure v2.x next to v1.x and other languages as easily. 

This change reverts the incidental change, to a more intentional one that preserves `slackclient`. One disadvantage of this solution is that the string "slackclient" can be seen as not self-descriptive - "What is slackclient? At least `slack` reflects the name of the Python module.". I think the reflecting the name of the module is not as useful as reflecting the name of the package. And to that end, "slackclient" is the name this package is distributed as. It's not perfect, and if we could start all over we would likely choose something different, but I still think its worth correcting the current issue now and looking at what it might take to rename this package in a separate issue (if that's something we want to do).

This change now includes unit test to verify that the language and package identifier are present in the User Agent.

### Requirements (place an `x` in each `[ ]`)

* [x] I've read and understood the [Contributing Guidelines](https://github.com/slackapi/python-slackclient/blob/master/.github/contributing.md) and have done my best effort to follow them.
* [x] I've read and agree to the [Code of Conduct](https://slackhq.github.io/code-of-conduct).

codecov[bot]: # [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=h1) Report
> Merging [#418](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=desc) into [master](https://codecov.io/gh/slackapi/python-slackclient/commit/8e1a88f4afbdec3a2f02e6370bf0ac8c973fc666?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/slackapi/python-slackclient/pull/418/graphs/tree.svg?width=650&token=F0O7meYmtD&height=150&src=pr)](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #418   +/-   ##
=======================================
  Coverage   56.94%   56.94%           
=======================================
  Files           6        6           
  Lines         734      734           
  Branches       42       42           
=======================================
  Hits          418      418           
  Misses        307      307           
  Partials        9        9
```


| [Impacted Files](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [slack/web/base\_client.py](https://codecov.io/gh/slackapi/python-slackclient/pull/418/diff?src=pr&el=tree#diff-c2xhY2svd2ViL2Jhc2VfY2xpZW50LnB5) | `84.41% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=footer). Last update [8e1a88f...2ab0cc9](https://codecov.io/gh/slackapi/python-slackclient/pull/418?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/documentation_v2/basic_usage.md b/documentation_v2/basic_usage.md
index 9a4c256..d6ebc1c 100755
--- a/documentation_v2/basic_usage.md
+++ b/documentation_v2/basic_usage.md
@@ -310,7 +310,7 @@ elif response["ok"] is False and response["headers"]["Retry-After"]:
 ```
 See the documentation on [Rate Limiting][rate-limiting] for more info.
 
-[auth]: basic_usage.md
+[auth]: auth.md
 [chat.postMessage]: https://api.slack.com/methods/chat.postMessage
 [chat.postEphemeral]: https://api.slack.com/methods/chat.postEphemeral
 [blockkit-builder]: https://api.slack.com/tools/block-kit-builder
@@ -325,4 +325,4 @@ See the documentation on [Rate Limiting][rate-limiting] for more info.
 [channels.leave]: https://api.slack.com/methods/channels.leave
 [users.list]: https://api.slack.com/methods/users.list
 [files.upload]: https://api.slack.com/methods/files.upload
-[rate-limiting]: https://api.slack.com/docs/rate-limits
\ No newline at end of file
+[rate-limiting]: https://api.slack.com/docs/rate-limits
diff --git a/slack/web/base_client.py b/slack/web/base_client.py
index 8ac7c64..24795e8 100644
--- a/slack/web/base_client.py
+++ b/slack/web/base_client.py
@@ -219,10 +219,10 @@ class BaseClient:
 
         Returns:
             The user agent string.
-            e.g. 'Python/3.6.7 slack/2.0.0 Darwin/17.7.0'
+            e.g. 'Python/3.6.7 slackclient/2.0.0 Darwin/17.7.0'
         """
         # __name__ returns all classes, we only want the client
-        client = "{0}/{1}".format(__name__.split(".")[0], ver.__version__)
+        client = "{0}/{1}".format("slackclient", ver.__version__)
         python_version = "Python/{v.major}.{v.minor}.{v.micro}".format(
             v=sys.version_info
         )


Test patch:
diff --git a/tests/web/test_web_client.py b/tests/web/test_web_client.py
index 6bf450b..2907bae 100644
--- a/tests/web/test_web_client.py
+++ b/tests/web/test_web_client.py
@@ -2,6 +2,7 @@
 import unittest
 from unittest import mock
 import asyncio
+import re
 
 
 # Internal Imports
@@ -18,11 +19,34 @@ class TestWebClient(unittest.TestCase):
     def tearDown(self):
         pass
 
+    pattern_for_language = re.compile("python/(\\S+)", re.IGNORECASE)
+    pattern_for_package_identifier = re.compile("slackclient/(\\S+)")
+
     def test_api_calls_return_a_response_when_run_in_sync_mode(self, mock_request):
         resp = self.client.api_test()
         self.assertFalse(asyncio.isfuture(resp))
         self.assertTrue(resp["ok"])
 
+    def test_api_calls_include_user_agent(self, mock_request):
+        self.client.api_test()
+        mock_call_kwargs = mock_request.call_args[1]
+        self.assertIn("req_args", mock_call_kwargs)
+        mock_call_req_args = mock_call_kwargs["req_args"]
+        self.assertIn("headers", mock_call_req_args)
+        mock_call_headers = mock_call_req_args["headers"]
+        self.assertIn("User-Agent", mock_call_headers)
+        mock_call_user_agent = mock_call_headers["User-Agent"]
+        self.assertRegex(
+            mock_call_user_agent,
+            self.pattern_for_package_identifier,
+            "User Agent contains slackclient and version",
+        )
+        self.assertRegex(
+            mock_call_user_agent,
+            self.pattern_for_language,
+            "User Agent contains Python and version",
+        )
+
     @async_test
     async def test_api_calls_return_a_future_when_run_in_async_mode(self, mock_request):
         self.client.run_async = True