Mypy error fixes #159

nforsg · 2023-08-10T09:57:02Z

Jobbar på och commitar med jämna mellanrum

Limmen · 2023-08-10T10:35:02Z

simulation-system/libs/csle-common/src/csle_common/metastore/metastore_facade.py

@@ -71,7 +71,7 @@ def list_emulations_ids() -> List[Tuple]:
                return records

    @staticmethod
-    def get_emulation_by_name(name: str) -> Union[None, EmulationEnvConfig]:
+    def get_emulation_by_name(name: Union[None, str]) -> Union[None, EmulationEnvConfig]:


Lägg bara till Union[None, originalType] om koden fungerar med None. I denna metoden, vill vi tillåta None? Antagligen inte, det är inte rimligt att hämta emulation_by_name på None. Då är det nog bättre att ha kvar name: str och försöka undvika None genom att lägga till "if name is not None, get_emulation_by_name(name)" och skicka ett exception om det är None där denna funktionen anropas

Limmen · 2023-08-10T10:37:00Z

simulation-system/libs/csle-common/src/csle_common/dao/docker/docker_container_metadata.py

@@ -8,9 +8,9 @@ class DockerContainerMetadata(JSONSerializable):
    """

    def __init__(self, name: str, status: str, short_id: str, image_short_id: str, image_tags: list, id: str,


Samma fall här, försök undvika None typer.

Jag gissar att mypy klagar för att någonstans anropas DockerContainerMetadata() med värden som kan vara None. För att ta bort det error utan att lägga till None typer kan du lägga till en exception/if-statement innan DockerContainerMetadata() anropas för att kolla efter None värden.

Sjävklart på alla ställen där vi redan har initialsierat argumenten till None så kan du lägga till None Union-typen. Men försök undvika att lägga till None typer på ställen där vi inte har kodat redan för att hantera None, då är det bättre att bara lägga in en exception eller if-case.

Limmen · 2023-08-10T10:38:04Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

@@ -23,7 +23,7 @@ def parse_runnning_emulation_infos() -> List[DockerEnvMetadata]:
        client_1 = docker.from_env()
        client_2 = docker.APIClient(base_url=constants.DOCKER.UNIX_DOCKER_SOCK_URL)
        parsed_containers = DockerUtil.parse_running_containers(client_1=client_1, client_2=client_2)
-        emulations = list(set(list(map(lambda x: x.emulation, parsed_containers))))
+        emulations: List[Union[None, str]] = list(set(list(map(lambda x: x.emulation, parsed_containers))))


Samma här, antagligen vill vi inte tillåta None. Det du kan göra är att filtrera listen för None först eller försök hitta roten till problemet där någonstans returneras None (antagligen i ett externt bibliotek) och där vill vi ha en if-statement/exception om det är None för att undvika att lägga till None i vår kod där det inte ska vara None.

Vill vi att det exception ska vara inne i init i den klassen isf? Typ:
if emulation is None: raise ValueError(...)
else: self.emulation = emulation
?

oh nvm såg din kommentar ovan

Limmen · 2023-08-10T10:38:17Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

@@ -72,7 +72,7 @@ def parse_stopped_containers(client_1, client2) -> List[DockerContainerMetadata]
        return parsed_containers

    @staticmethod
-    def parse_running_emulation_envs(emulations: List[str], containers: List[DockerContainerMetadata]) \
+    def parse_running_emulation_envs(emulations: List[Union[None, str]], containers: List[DockerContainerMetadata]) \


Vill nog inte tillåta None här

Limmen · 2023-08-10T10:38:23Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

@@ -180,7 +180,7 @@ def get_docker_gw_bridge_ip(container_id: str) -> str:
        containers = docker_gw_bridge_info[constants.DOCKER.CONTAINERS_KEY]
        if container_id in containers:
            container = containers[container_id]
-            ip = container[constants.DOCKER.IPV4_KEY]
+            ip: str = container[constants.DOCKER.IPV4_KEY]


Limmen · 2023-08-10T10:38:37Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

        with os.scandir(dir_path) as it:
            for entry in it:
                if entry.is_file():
                    total += entry.stat().st_size
                elif entry.is_dir():
-                    total += ExportUtil.get_dir_size(entry.path)
+                    total += ExportUtil.get_dir_size_gb(entry.path)


Limmen · 2023-08-10T10:38:46Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

@@ -111,7 +113,7 @@ def export_emulation_traces_to_disk_json(num_traces_per_file: int, output_dir: s
        num_traces = len(emulation_traces_ids)
        with io.open(f"{output_dir}{constants.COMMANDS.SLASH_DELIM}{constants.DATASETS.METADATA_FILE_NAME}", 'w',
                     encoding='utf-8') as f:
-            metadata_dict = {}
+            metadata_dict: Dict[str, Any] = {}


Limmen · 2023-08-10T10:38:49Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

@@ -138,7 +140,7 @@ def extract_emulation_traces_dataset_metadata(dir_path: str, zip_file_path: str)
        file_format = "unknown"
        added_by = "unknown"
        num_traces = -1
-        schema = ""
+        schema = {}


Limmen · 2023-08-10T10:39:00Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

@@ -213,10 +215,12 @@ def export_emulation_traces_to_disk_csv(
        file_name = f"{file_id}.csv"
        last_export = 0
        num_attributes_per_time_step = -1
-        columns = None
+        columns: Union[List[Union[str, int, float]], None]= None


Limmen · 2023-08-10T10:39:11Z

simulation-system/libs/csle-common/src/csle_common/util/import_util.py


 class ImportUtil:
    """
    Class with utility functions for importing data to the metastore
    """

    @staticmethod
-    def import_emulation_statistics_from_disk_json(input_file: str, emulation_name: str = None) -> None:
+    def import_emulation_statistics_from_disk_json(input_file: str, emulation_name: Union[str, None] = None) -> None:


nforsg · 2023-08-10T11:34:27Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

Vet inte om det var såhär du menade?

Limmen · 2023-08-10T11:43:54Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

@@ -87,6 +87,8 @@ def export_emulation_traces_to_disk_json(num_traces_per_file: int, output_dir: s
            Logger.__call__().get_logger().info(f"Reading trace {i}/{len(emulation_traces_ids)} from the metastore, "
                                                f"trace id: {id_obj[0]}")
            tr = MetastoreFacade.get_emulation_trace(id=id_obj[0])
+            if tr is None:


Bra, men använd alltid deskiptiva variabel namn. I testcases var det ok med lite abbreviations, men inte i main-koden. Dvs det ska inte vara tr utan "trace" i detta fallet. https://readlearncode.com/code-and-stuff/clean-code-variables/

Limmen · 2023-08-10T11:44:08Z

simulation-system/libs/csle-common/src/csle_common/util/export_util.py

@@ -213,10 +215,12 @@ def export_emulation_traces_to_disk_csv(
        file_name = f"{file_id}.csv"
        last_export = 0
        num_attributes_per_time_step = -1
-        columns = None
+        columns: Union[List[Union[str, int, float]], None]= None
        for i, id_obj in enumerate(emulation_traces_ids):
            Logger.__call__().get_logger().info(f"Reading trace {i}/{len(emulation_traces_ids)} from the metastore")
            tr = MetastoreFacade.get_emulation_trace(id=id_obj[0])


trace inte tr

Limmen · 2023-08-10T11:45:16Z

simulation-system/libs/csle-common/src/csle_common/dao/docker/docker_container_metadata.py

                 name2: str, level: str, hostname: str, image_name: str, net: str,
-                 dir: str, config_path: str, container_handle: str, emulation: str, kafka_container: str):
+                 dir: Union[str, None], config_path: Union[None, str], container_handle: str, emulation: str, kafka_container: Union[None, str]):


Några Union[None, typ]. Går det att undvika dem på ett enkelt sätt.

Nevermind, kollade det precis och jag tror inte vi kan undvika None här, None är OK

Limmen · 2023-08-10T11:47:51Z

simulation-system/libs/csle-common/src/csle_common/dao/docker/docker_env_metadata.py

@@ -11,7 +11,7 @@ class DockerEnvMetadata(JSONSerializable):
    """

    def __init__(self, containers: List[DockerContainerMetadata], name: str, subnet_prefix: str,


Kafka config None går inte att undvika här, men config bör vi nog inte tillåta vara None.

Uppdatera även to_dict och from_dict i docker_env_metadata så att de inte failar om kafka_config är None

Limmen · 2023-08-10T11:49:35Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

Limmen · 2023-08-10T11:55:03Z

simulation-system/libs/csle-common/src/csle_common/util/docker_util.py

+        for container in parsed_containers:
+            if container is None:
+                raise ValueError("container canot be None")
+        emulations: List[str] = list(set(list(map(lambda x: x.emulation, parsed_containers))))


Suggested change

emulations: List[str] = list(set(list(map(lambda x: x.emulation, parsed_containers))))

Ändra till

```suggestion

emulations: List[str] = list(set(list(map(lambda x: x.emulation, filter(lambda x: x is not None, parsed_containers)))))

nforsg · 2023-08-10T12:08:07Z

simulation-system/libs/csle-common/src/csle_common/metastore/metastore_facade.py

Nästan alla list-funktioner i metastorefacade går felmeddelandet att dom returnerar List[Tuple[Any, ...]] när dom ska returnera List[ClassConfigObject],. Eftersom jag har mockat alla fuinktioner efter den type-hintinhen är jag inte säker på att det rätt att ändra till List[Tuple[Any, ...]] på dom funktionerna?

såhär tycker mypy att det ska se ut på dessa sorters funktioner, vilket då inte stämmer överens med hur jag mockat. Ska jag köra på det iallafall?

Limmen · 2023-08-10T12:08:17Z

simulation-system/libs/csle-common/src/csle_common/metastore/metastore_facade.py

@@ -57,7 +57,7 @@ def list_emulations() -> List[EmulationEnvConfig]:
                return records

    @staticmethod
-    def list_emulations_ids() -> List[Tuple]:
+    def list_emulations_ids() -> List[Tuple[str, int]]:


Limmen · 2023-08-10T12:09:12Z

simulation-system/libs/csle-common/src/csle_common/metastore/metastore_facade.py

@@ -124,7 +124,7 @@ def list_simulations() -> List[SimulationEnvConfig]:
                return records


Kan vi vara mer specifika här? Borde det inte vara List[Tuple[str, int]]? Du kan skapa en testfil och testköra metoden och se vad du får tillbaka för typ

nforsg added 2 commits August 10, 2023 11:09

errors on row 106-109 are hard to solve

49546dd

fixed docker_util.py errors

72ca5f4

Limmen requested changes Aug 10, 2023

View reviewed changes

improved error handling

6f662e3

nforsg commented Aug 10, 2023

View reviewed changes

Limmen requested changes Aug 10, 2023

View reviewed changes

Limmen reviewed Aug 10, 2023

View reviewed changes

improved error handling

a915a78

nforsg commented Aug 10, 2023

View reviewed changes

Limmen reviewed Aug 10, 2023

View reviewed changes

nforsg added 5 commits August 10, 2023 14:09

demo

2dcaf2a

doing this commit to be able to revert

fdb4f5d

to be able to revert to previous

899d438

trying'

e7e9b66

triyng

c422b14

Limmen closed this Aug 10, 2023

nforsg deleted the dev branch August 10, 2023 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mypy error fixes #159

Mypy error fixes #159

nforsg commented Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

nforsg Aug 10, 2023

nforsg Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

nforsg Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

nforsg Aug 10, 2023

nforsg Aug 10, 2023

Limmen Aug 10, 2023

Limmen Aug 10, 2023

		@@ -8,9 +8,9 @@ class DockerContainerMetadata(JSONSerializable):
		"""

		def __init__(self, name: str, status: str, short_id: str, image_short_id: str, image_tags: list, id: str,

		@@ -11,7 +11,7 @@ class DockerEnvMetadata(JSONSerializable):
		"""

		def __init__(self, containers: List[DockerContainerMetadata], name: str, subnet_prefix: str,

-        emulations: List[str] = list(set(list(map(lambda x: x.emulation, parsed_containers))))
+        Ändra till
+```suggestion
+        emulations: List[str] = list(set(list(map(lambda x: x.emulation, filter(lambda x: x is not None, parsed_containers)))))

		@@ -124,7 +124,7 @@ def list_simulations() -> List[SimulationEnvConfig]:
		return records

Mypy error fixes #159

Mypy error fixes #159

Conversation

nforsg commented Aug 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment