Repository: microsoft/botbuilder-python
Pull request #267 (2019-07-19 21:45:34)

azure pylint issues
Pylint fixes for azure library code and tests


Patch:
diff --git a/libraries/botbuilder-azure/botbuilder/azure/cosmosdb_storage.py b/libraries/botbuilder-azure/botbuilder/azure/cosmosdb_storage.py
index b55ef66c..c85901ae 100644
--- a/libraries/botbuilder-azure/botbuilder/azure/cosmosdb_storage.py
+++ b/libraries/botbuilder-azure/botbuilder/azure/cosmosdb_storage.py
@@ -10,9 +10,9 @@ from hashlib import sha256
 from typing import Dict, List
 from threading import Semaphore
 import json
+import azure.cosmos.cosmos_client as cosmos_client  # pylint: disable=no-name-in-module,import-error
+import azure.cosmos.errors as cosmos_errors  # pylint: disable=no-name-in-module,import-error
 from botbuilder.core.storage import Storage, StoreItem
-import azure.cosmos.cosmos_client as cosmos_client
-import azure.cosmos.errors as cosmos_errors
 
 
 class CosmosDbConfig:
@@ -74,13 +74,13 @@ class CosmosDbKeyEscape:
 
     @staticmethod
     def truncate_key(key: str) -> str:
-        MAX_KEY_LEN = 255
+        max_key_len = 255
 
-        if len(key) > MAX_KEY_LEN:
+        if len(key) > max_key_len:
             aux_hash = sha256(key.encode("utf-8"))
             aux_hex = aux_hash.hexdigest()
 
-            key = key[0 : MAX_KEY_LEN - len(aux_hex)] + aux_hex
+            key = key[0 : max_key_len - len(aux_hex)] + aux_hex
 
         return key
 
@@ -101,8 +101,8 @@ class CosmosDbStorage(Storage):
             self.config.endpoint, {"masterKey": self.config.masterkey}
         )
         # these are set by the functions that check
-        # the presence of the db and container or creates them
-        self.db = None
+        # the presence of the database and container or creates them
+        self.database = None
         self.container = None
         self._database_creation_options = config.database_creation_options
         self._container_creation_options = config.container_creation_options
@@ -146,11 +146,11 @@ class CosmosDbStorage(Storage):
                 )
                 # return a dict with a key and a StoreItem
                 return {r.get("realId"): self.__create_si(r) for r in results}
-            else:
-                # No keys passed in, no result to return.
-                return {}
-        except TypeError as e:
-            raise e
+
+            # No keys passed in, no result to return.
+            return {}
+        except TypeError as error:
+            raise error
 
     async def write(self, changes: Dict[str, StoreItem]):
         """Save storeitems to storage.
@@ -180,7 +180,7 @@ class CosmosDbStorage(Storage):
                         options={"disableAutomaticIdGeneration": True},
                     )
                 # if we have an etag, do opt. concurrency replace
-                elif len(e_tag) > 0:
+                elif e_tag:
                     access_condition = {"type": "IfMatch", "condition": e_tag}
                     self.client.ReplaceItem(
                         document_link=self.__item_link(
@@ -192,8 +192,8 @@ class CosmosDbStorage(Storage):
                 # error when there is no e_tag
                 else:
                     raise Exception("cosmosdb_storage.write(): etag missing")
-        except Exception as e:
-            raise e
+        except Exception as error:
+            raise error
 
     async def delete(self, keys: List[str]):
         """Remove storeitems from storage.
@@ -211,18 +211,18 @@ class CosmosDbStorage(Storage):
                 options["partitionKey"] = self.config.partition_key
 
             # call the function for each key
-            for k in keys:
+            for key in keys:
                 self.client.DeleteItem(
-                    document_link=self.__item_link(CosmosDbKeyEscape.sanitize_key(k)),
+                    document_link=self.__item_link(CosmosDbKeyEscape.sanitize_key(key)),
                     options=options,
                 )
                 # print(res)
-        except cosmos_errors.HTTPFailure as h:
+        except cosmos_errors.HTTPFailure as http_failure:
             # print(h.status_code)
-            if h.status_code != 404:
-                raise h
-        except TypeError as e:
-            raise e
+            if http_failure.status_code != 404:
+                raise http_failure
+        except TypeError as error:
+            raise error
 
     def __create_si(self, result) -> StoreItem:
         """Create a StoreItem from a result out of CosmosDB.
@@ -238,28 +238,28 @@ class CosmosDbStorage(Storage):
         # create and return the StoreItem
         return StoreItem(**doc)
 
-    def __create_dict(self, si: StoreItem) -> Dict:
+    def __create_dict(self, store_item: StoreItem) -> Dict:
         """Return the dict of a StoreItem.
 
         This eliminates non_magic attributes and the e_tag.
 
-        :param si:
+        :param store_item:
         :return dict:
         """
         # read the content
         non_magic_attr = [
-            attr for attr in dir(si) if not attr.startswith("_") or attr.__eq__("e_tag")
+            attr for attr in dir(store_item) if not attr.startswith("_") or attr.__eq__("e_tag")
         ]
         # loop through attributes and write and return a dict
-        return {attr: getattr(si, attr) for attr in non_magic_attr}
+        return {attr: getattr(store_item, attr) for attr in non_magic_attr}
 
-    def __item_link(self, id) -> str:
+    def __item_link(self, identifier) -> str:
         """Return the item link of a item in the container.
 
-        :param id:
+        :param identifier:
         :return str:
         """
-        return self.__container_link + "/docs/" + id
+        return self.__container_link + "/docs/" + identifier
 
     @property
     def __container_link(self) -> str:
@@ -276,7 +276,7 @@ class CosmosDbStorage(Storage):
 
         :return str:
         """
-        return "dbs/" + self.db
+        return "dbs/" + self.database
 
     @property
     def __container_exists(self) -> bool:
@@ -284,20 +284,20 @@ class CosmosDbStorage(Storage):
 
         :return bool:
         """
-        return self.db and self.container
+        return self.database and self.container
 
     def __create_db_and_container(self):
         """Call the get or create methods."""
         with self.__semaphore:
             db_id = self.config.database
             container_name = self.config.container
-            self.db = self._get_or_create_database(self.client, db_id)
+            self.database = self._get_or_create_database(self.client, db_id)
             self.container = self._get_or_create_container(self.client, container_name)
 
-    def _get_or_create_database(self, doc_client, id) -> str:
+    def _get_or_create_database(self, doc_client, id) -> str:  # pylint: disable=invalid-name
         """Return the database link.
 
-        Check if the database exists or create the db.
+        Check if the database exists or create the database.
 
         :param doc_client:
         :param id:
@@ -312,13 +312,13 @@ class CosmosDbStorage(Storage):
                 }
             )
         )
-        # if there are results, return the first (db names are unique)
-        if len(dbs) > 0:
+        # if there are results, return the first (database names are unique)
+        if dbs:
             return dbs[0]["id"]
-        else:
-            # create the database if it didn't exist
-            res = doc_client.CreateDatabase({"id": id}, self._database_creation_options)
-            return res["id"]
+
+        # create the database if it didn't exist
+        res = doc_client.CreateDatabase({"id": id}, self._database_creation_options)
+        return res["id"]
 
     def _get_or_create_container(self, doc_client, container) -> str:
         """Return the container link.
@@ -340,13 +340,13 @@ class CosmosDbStorage(Storage):
             )
         )
         # if there are results, return the first (container names are unique)
-        if len(containers) > 0:
+        if containers:
             return containers[0]["id"]
-        else:
-            # Create a container if it didn't exist
-            res = doc_client.CreateContainer(
-                self.__database_link,
-                {"id": container},
-                self._container_creation_options,
-            )
-            return res["id"]
+
+        # Create a container if it didn't exist
+        res = doc_client.CreateContainer(
+            self.__database_link,
+            {"id": container},
+            self._container_creation_options,
+        )
+        return res["id"]


Test patch:
diff --git a/libraries/botbuilder-azure/tests/test_cosmos_storage.py b/libraries/botbuilder-azure/tests/test_cosmos_storage.py
index 25f5474c..7e686f20 100644
--- a/libraries/botbuilder-azure/tests/test_cosmos_storage.py
+++ b/libraries/botbuilder-azure/tests/test_cosmos_storage.py
@@ -1,44 +1,45 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
+from unittest.mock import Mock
 import azure.cosmos.errors as cosmos_errors
-import pytest
 from azure.cosmos.cosmos_client import CosmosClient
+import pytest
 from botbuilder.core import StoreItem
 from botbuilder.azure import CosmosDbStorage, CosmosDbConfig
-from unittest.mock import Mock
 
 # local cosmosdb emulator instance cosmos_db_config
-cosmos_db_config = CosmosDbConfig(
+COSMOS_DB_CONFIG = CosmosDbConfig(
     endpoint="https://localhost:8081",
     masterkey="C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
     database="test-db",
     container="bot-storage",
 )
-emulator_running = False
+EMULATOR_RUNNING = False
 
 
 async def reset():
-    storage = CosmosDbStorage(cosmos_db_config)
+    storage = CosmosDbStorage(COSMOS_DB_CONFIG)
     try:
-        storage.client.DeleteDatabase(database_link="dbs/" + cosmos_db_config.database)
+        storage.client.DeleteDatabase(database_link="dbs/" + COSMOS_DB_CONFIG.database)
     except cosmos_errors.HTTPFailure:
         pass
 
 
-def get_mock_client(id: str = "1"):
+def get_mock_client(identifier: str = "1"):
+    # pylint: disable=invalid-name
     mock = MockClient()
 
     mock.QueryDatabases = Mock(return_value=[])
     mock.QueryContainers = Mock(return_value=[])
-    mock.CreateDatabase = Mock(return_value={"id": id})
-    mock.CreateContainer = Mock(return_value={"id": id})
+    mock.CreateDatabase = Mock(return_value={"id": identifier})
+    mock.CreateContainer = Mock(return_value={"id": identifier})
 
     return mock
 
 
 class MockClient(CosmosClient):
-    def __init__(self):
+    def __init__(self):  # pylint: disable=super-init-not-called
         pass
 
 
@@ -54,11 +55,12 @@ class TestCosmosDbStorage:
     async def test_cosmos_storage_init_should_error_without_cosmos_db_config(self):
         try:
             CosmosDbStorage(CosmosDbConfig())
-        except Exception as e:
-            assert e
+        except Exception as error:
+            assert error
 
     @pytest.mark.asyncio
     async def test_creation_request_options_era_being_called(self):
+        # pylint: disable=protected-access
         test_config = CosmosDbConfig(
             endpoint="https://localhost:8081",
             masterkey="C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
@@ -69,9 +71,9 @@ class TestCosmosDbStorage:
         )
 
         test_id = "1"
-        client = get_mock_client(id=test_id)
+        client = get_mock_client(identifier=test_id)
         storage = CosmosDbStorage(test_config, client)
-        storage.db = test_id
+        storage.database = test_id
 
         assert storage._get_or_create_database(doc_client=client, id=test_id), test_id
         client.CreateDatabase.assert_called_with(
@@ -84,12 +86,12 @@ class TestCosmosDbStorage:
             "dbs/" + test_id, {"id": test_id}, test_config.container_creation_options
         )
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_init_should_work_with_just_endpoint_and_key(self):
         storage = CosmosDbStorage(
             CosmosDbConfig(
-                endpoint=cosmos_db_config.endpoint, masterkey=cosmos_db_config.masterkey
+                endpoint=COSMOS_DB_CONFIG.endpoint, masterkey=COSMOS_DB_CONFIG.masterkey
             )
         )
         await storage.write({"user": SimpleStoreItem()})
@@ -98,11 +100,11 @@ class TestCosmosDbStorage:
         assert data["user"].counter == 1
         assert len(data.keys()) == 1
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_read_should_return_data_with_valid_key(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"user": SimpleStoreItem()})
 
         data = await storage.read(["user"])
@@ -110,11 +112,11 @@ class TestCosmosDbStorage:
         assert data["user"].counter == 1
         assert len(data.keys()) == 1
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_read_update_should_return_new_etag(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"test": SimpleStoreItem(counter=1)})
         data_result = await storage.read(["test"])
         data_result["test"].counter = 2
@@ -123,55 +125,55 @@ class TestCosmosDbStorage:
         assert data_updated["test"].counter == 2
         assert data_updated["test"].e_tag != data_result["test"].e_tag
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_read_with_invalid_key_should_return_empty_dict(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         data = await storage.read(["test"])
 
-        assert type(data) == dict
-        assert len(data.keys()) == 0
+        assert isinstance(data, dict)
+        assert not data.keys()
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_read_no_key_should_throw(self):
         try:
             await reset()
-            storage = CosmosDbStorage(cosmos_db_config)
+            storage = CosmosDbStorage(COSMOS_DB_CONFIG)
             await storage.read([])
-        except Exception as e:
-            assert e
+        except Exception as error:
+            assert error
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_write_should_add_new_value(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"user": SimpleStoreItem(counter=1)})
 
         data = await storage.read(["user"])
         assert "user" in data
         assert data["user"].counter == 1
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_write_should_overwrite_when_new_e_tag_is_an_asterisk(
         self
     ):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"user": SimpleStoreItem()})
 
         await storage.write({"user": SimpleStoreItem(counter=10, e_tag="*")})
         data = await storage.read(["user"])
         assert data["user"].counter == 10
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_write_batch_operation(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write(
             {
                 "batch1": SimpleStoreItem(counter=1),
@@ -192,13 +194,13 @@ class TestCosmosDbStorage:
         assert data["batch3"].e_tag
         await storage.delete(["batch1", "batch2", "batch3"])
         data = await storage.read(["batch1", "batch2", "batch3"])
-        assert len(data.keys()) == 0
+        assert not data.keys()
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_write_crazy_keys_work(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         crazy_key = '!@#$%^&*()_+??><":QASD~`'
         await storage.write({crazy_key: SimpleStoreItem(counter=1)})
         data = await storage.read([crazy_key])
@@ -207,42 +209,42 @@ class TestCosmosDbStorage:
         assert data[crazy_key].counter == 1
         assert data[crazy_key].e_tag
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_delete_should_delete_according_cached_data(self):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"test": SimpleStoreItem()})
         try:
             await storage.delete(["test"])
-        except Exception as e:
-            raise e
+        except Exception as error:
+            raise error
         else:
             data = await storage.read(["test"])
 
-            assert type(data) == dict
-            assert len(data.keys()) == 0
+            assert isinstance(data, dict)
+            assert not data.keys()
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_delete_should_delete_multiple_values_when_given_multiple_valid_keys(
         self
     ):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"test": SimpleStoreItem(), "test2": SimpleStoreItem(2)})
 
         await storage.delete(["test", "test2"])
         data = await storage.read(["test", "test2"])
-        assert len(data.keys()) == 0
+        assert not data.keys()
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_delete_should_delete_values_when_given_multiple_valid_keys_and_ignore_other_data(
         self
     ):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write(
             {
                 "test": SimpleStoreItem(),
@@ -255,28 +257,28 @@ class TestCosmosDbStorage:
         data = await storage.read(["test", "test2", "test3"])
         assert len(data.keys()) == 1
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_delete_invalid_key_should_do_nothing_and_not_affect_cached_data(
         self
     ):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"test": SimpleStoreItem()})
 
         await storage.delete(["foo"])
         data = await storage.read(["test"])
         assert len(data.keys()) == 1
         data = await storage.read(["foo"])
-        assert len(data.keys()) == 0
+        assert not data.keys()
 
-    @pytest.mark.skipif(not emulator_running, reason="Needs the emulator to run.")
+    @pytest.mark.skipif(not EMULATOR_RUNNING, reason="Needs the emulator to run.")
     @pytest.mark.asyncio
     async def test_cosmos_storage_delete_invalid_keys_should_do_nothing_and_not_affect_cached_data(
         self
     ):
         await reset()
-        storage = CosmosDbStorage(cosmos_db_config)
+        storage = CosmosDbStorage(COSMOS_DB_CONFIG)
         await storage.write({"test": SimpleStoreItem()})
 
         await storage.delete(["foo", "bar"])