Dev python exercise 1 #10
Conversation
solutions/python/exercise1/backup.py
Outdated
| def main() -> None: | ||
| """Connect to devices and save the running config to files as backup.""" | ||
| logging.basicConfig(level=logging.INFO) | ||
| devices_info = read_yaml("hosts.yaml") |
There was a problem hiding this comment.
A lot of frameworks in the networking automation refer to structures like that as inventory so it's good to follow that convention. Naming variables is hard :)
There was a problem hiding this comment.
Changed it to inventory. And yes, naming variables is a tough business 😨
solutions/python/exercise1/backup.py
Outdated
| try: | ||
| logging.info(f"Read data from { file_name }") | ||
| with open(file_name, encoding="UTF-8") as f: | ||
| yaml_data = yaml.load(f, Loader=FullLoader) |
There was a problem hiding this comment.
Use SafeLoader instead of FullLoader. FullLoader has known vulnerabilities, see yaml/pyyaml#420 for examples.
Only use FullLoader if absolutely trust the data and SafeLoader is not working with your data structure.
solutions/python/exercise1/backup.py
Outdated
| dict: data from yaml file | ||
| """ | ||
| try: | ||
| logging.info(f"Read data from { file_name }") |
There was a problem hiding this comment.
I'd say this is more of a debug level message than info.
solutions/python/exercise1/backup.py
Outdated
| device_info["username"] = USERNAME | ||
| device_info["password"] = PASSWORD | ||
| device_output = send_command( | ||
| device=device_info, command=COMMANDS[device_info["device_type"]] |
There was a problem hiding this comment.
.get() is usually safer in situations like this. This also allows you to provide the default value. E.g.
... command=COMMANDS.get(device_info["device_type"], "show running-config")
solutions/python/exercise1/backup.py
Outdated
| file_dir = Path(file_name).parent.absolute() | ||
| if not file_dir.exists(): | ||
| logging.info(f"Creating directory { file_dir } for backups") | ||
| file_dir.mkdir(parents=True) |
There was a problem hiding this comment.
This should be probably moved into it's own function called once during the setup.
There was a problem hiding this comment.
It was my initial approach and changed it, changed it back to have it's own function.
solutions/python/exercise1/backup.py
Outdated
| logging.info(f"Read data from { file_name }") | ||
| with open(file_name, encoding="UTF-8") as f: | ||
| yaml_data = yaml.load(f, Loader=FullLoader) | ||
| except IOError as err: |
There was a problem hiding this comment.
Catch OSError instead. IOError was aliased to OSError starting wtih Python3.3.
solutions/python/exercise1/backup.py
Outdated
| file_dir = Path(file_name).parent.absolute() | ||
| if not file_dir.exists(): | ||
| logging.info(f"Creating directory { file_dir } for backups") | ||
| file_dir.mkdir(parents=True) |
There was a problem hiding this comment.
You can user mkdir with exist_ok=True if you want to avoid the if check.
No description provided.