import pefile  # type: ignore
from datetime import datetime
import logging


def get_portable_executable_info(file_path: str) -> dict:
    Return a dictionary with ProductVersion and other params of portable executable.

    The Portable Executable (PE) format is a file format for executables, object code, DLLs and others used in 32-bit
    and 64-bit versions of Windows operating systems. For non PE files this error is raised:
    pefile.PEFormatError: 'DOS Header magic not found.'
    We can use it to provide the info about the tested build to the pytest report.
    decoded_dict: dict = {}  # To avoid "Local variable 'pe' might be referenced before assignment" alert.
        pe = pefile.PE(file_path)  # type - class 'pefile.PE'
    except Exception as error:
        return decoded_dict

    file_info: list = pe.FileInfo  # In my case: List of lists. len==1.
    # The nested list contains 2 elements of class 'pefile.Structure': VarFileInfo and StringFileInfo

    for structure in file_info[0]:
        logging.debug(f"This structure type is {type(structure)}")
        if hasattr(structure, 'StringTable'):
            logging.debug("This entry CONTAINS attribute StringTable.")
            string_table: list = structure.StringTable  # In my case:len==1.
            # Contains 1 element of class 'pefile.Structure'
            encoded_dict: dict = string_table[0].entries
            decoded_dict = dict(
                (key.decode('utf-8'), value.decode('utf-')) for key, value in encoded_dict.items())
            logging.debug("This entry doesn't contain attribute StringTable.")

    # Add modification time from FILE_HEADER
    time_date_stamp: int = pe.FILE_HEADER.TimeDateStamp
    modification_time: str = str(datetime.fromtimestamp(time_date_stamp))
    decoded_dict["Modification_time"] = modification_time
    return decoded_dict

 Public There are many comments in this code - just for debugging purposes.

Share a link to this review

10.87% issue ratio

L26 Executing code in global scope

When you have some code in module scope, the code will be executed when someone imports that module - which makes the module completely non-reusable. Maybe it's not meant to be reusable today, but you'll have to refactor if you need to import the module later. Use if __name__ == '__main__' to detect whether module was imported or used as a standalone script.

Better put logging setup inside if __name__ == '__main__' section

L56 Catching generic exception

Catching Exception or everything means that you don't understand what the code does. So you may catch and suppress exceptions which you probably didn't want to catch at all but you weren't aware of them. It is better for your program to fail, rather then catching everything.

R14 Not using "early quit"

"Early quit" is a common pattern which reduces branching and indents in python code. Instead of writing if a: <1000 lines of code> else: <return / raise exception>, revert the condition: if not a: <return / raise exception>. This way the "else" clause is not needed, and program logic is straightforward: if something is wrong, quit or raise, otherwise go on.

L68 Typo

People make typos. Here it is.

utf- -> utf-8

L12 Redundant code / overengineering

This code is not really needed or may be simplified

This is a counter-example of using type hints: here they don't add any value (especially modification_time: str = str(...) one), but make the code more polluted and less readable. Usually one should not annotate variables inside a function, unless it's hard to understand what's inside.

Create new review request