LCVcode / jockey

MIT License
2 stars 3 forks source link

Implement Poetry as the development lifecycle tool #32

Closed johnlettman closed 3 weeks ago

johnlettman commented 3 weeks ago

Et voila, this ends my PR spam for tonight.

This PR moves the repository to the Poetry Python dependency management and packaging system. Poetry helps manage the various development tools, keeping everything in one TOML config: https://python-poetry.org/

In other projects, I use it for relatively comprehensive workflows to check everything: https://github.com/johnlettman/jujucvetool/actions/runs/10425763665

Later down the road, this will also help with packaging.

johnlettman commented 3 weeks ago

It's worth noting the tools may need to be tweaked. It contains my opinionated modifications to 120-character lines, ditching the claustrophobic PEP8 80 characters.

As a result, the linter edits cause the following diff:

diff for `jockey.py` ```diff diff --git a/jockey.py b/jockey.py index 9a94737..df45d87 100755 --- a/jockey.py +++ b/jockey.py @@ -3,28 +3,15 @@ # match given filters. # Author: Connor Chamberlain -import pdb import argparse -import json -import re from dataclasses import dataclass from enum import Enum -from typing import ( - Any, - Dict, - NamedTuple, - Generator, - Optional, - List, - Tuple, - Iterable, -) +import json +import pdb +import re +from typing import Any, Dict, Generator, Iterable, List, NamedTuple, Optional, Tuple -from status_keeper import ( - retrieve_juju_cache, - cache_juju_status, - read_local_juju_status_file, -) +from status_keeper import cache_juju_status, read_local_juju_status_file, retrieve_juju_cache JujuStatus = Dict[str, Any] @@ -62,10 +49,7 @@ def list_abbreviations() -> str: pad = 15 header = "OBJECT TYPE".ljust(pad, " ") + "SHORT NAMES" - lines = [ - obj_type.value[0].ljust(pad, " ") + ", ".join(obj_type.value[1:]) - for obj_type in ObjectType - ] + lines = [obj_type.value[0].ljust(pad, " ") + ", ".join(obj_type.value[1:]) for obj_type in ObjectType] return "\n".join([header, *lines]) @@ -172,9 +156,7 @@ def convert_object_abbreviation(abbrev: str) -> Optional[ObjectType]: The ObjectType corresponding with the given abbrevation, if any. """ abbrev = abbrev.lower() - return next( - (obj_type for obj_type in ObjectType if abbrev in obj_type.value), None - ) + return next((obj_type for obj_type in ObjectType if abbrev in obj_type.value), None) def parse_filter_string( @@ -205,9 +187,7 @@ def parse_filter_string( object_type = convert_object_abbreviation(filter_str[: match.start()]) assert object_type, "Invalid object type detected in filter string." - filter_mode = next( - (mode for mode in FilterMode if mode.value == match.group()), None - ) + filter_mode = next((mode for mode in FilterMode if mode.value == match.group()), None) assert filter_mode, f"Invalid filter mode detected: {match.group()}." content = filter_str[match.end() :] @@ -249,9 +229,7 @@ def check_filter_match(jockey_filter: JockeyFilter, value: str) -> bool: return action(jockey_filter.content, value) -def check_filter_batch_match( - filter_list: Iterable[JockeyFilter], batch: Iterable[str] -) -> bool: +def check_filter_batch_match(filter_list: Iterable[JockeyFilter], batch: Iterable[str]) -> bool: """ Check if a batch of Juju objects (as strings) satisfies a set of filters. @@ -308,9 +286,7 @@ def is_app_principal(status: JujuStatus, app_name: str) -> bool: return "subordinate-to" not in status["applications"][app_name] -def get_principal_unit_for_subordinate( - status: JujuStatus, unit_name: str -) -> str: +def get_principal_unit_for_subordinate(status: JujuStatus, unit_name: str) -> str: """Get the name of a princpal unit for a given subordinate unit.""" for app, data in status["applications"].items(): @@ -460,9 +436,7 @@ def get_ips(status: JujuStatus) -> Generator[str, None, None]: yield address -def charm_to_applications( - status: JujuStatus, charm_name: str -) -> Generator[str, None, None]: +def charm_to_applications(status: JujuStatus, charm_name: str) -> Generator[str, None, None]: """ Given a charm name, get all applications using it, as a generator. If no matching charm is found, the generator will be empty. @@ -507,9 +481,7 @@ def application_to_charm(status: JujuStatus, app_name: str) -> Optional[str]: return None -def application_to_units( - status: JujuStatus, app_name: str -) -> Generator[str, None, None]: +def application_to_units(status: JujuStatus, app_name: str) -> Generator[str, None, None]: """ Given an application name, get all of its untis, as a generator. If no matching application is found, the generator will be empty. @@ -560,9 +532,7 @@ def unit_to_application(status: JujuStatus, unit_name: str) -> Optional[str]: return None -def subordinate_unit_to_principal_unit( - status: JujuStatus, unit_name: str -) -> str: +def subordinate_unit_to_principal_unit(status: JujuStatus, unit_name: str) -> str: """ Given a unit name, get its principal unit. If the given unit is principal, it will be returned as-is. @@ -619,9 +589,7 @@ def unit_to_machine(status: JujuStatus, unit_name: str) -> Optional[str]: return status["applications"][app]["units"][principal_unit_name]["machine"] -def machine_to_units( - status: JujuStatus, machine: str -) -> Generator[str, None, None]: +def machine_to_units(status: JujuStatus, machine: str) -> Generator[str, None, None]: """ Given an machine id, get all of its units, as a generator. If no matching units are found, the generator will be empty. @@ -653,15 +621,11 @@ def machine_to_units( if "subordinates" not in status["applications"][app]["units"][unit]: continue - for subordinate_unit in status["applications"][app]["units"][unit][ - "subordinates" - ]: + for subordinate_unit in status["applications"][app]["units"][unit]["subordinates"]: yield subordinate_unit -def machine_to_ips( - status: JujuStatus, machine: str -) -> Generator[str, None, None]: +def machine_to_ips(status: JujuStatus, machine: str) -> Generator[str, None, None]: """ Given an machine id, each of its IP addresses as a geneator. @@ -727,9 +691,7 @@ def machine_to_hostname(status: JujuStatus, machine: str) -> str: """ if "lxd" in machine: physical_machine, _, container_id = machine.split("/") - return status["machines"][physical_machine]["containers"][machine][ - "hostname" - ] + return status["machines"][physical_machine]["containers"][machine]["hostname"] return status["machines"][machine]["hostname"] @@ -756,9 +718,7 @@ def hostname_to_machine(status: JujuStatus, hostname: str) -> str: raise Exception(f"No machine found for hostname {hostname}") -def filter_units( - status: JujuStatus, filters: List[JockeyFilter] -) -> Generator[str, None, None]: +def filter_units(status: JujuStatus, filters: List[JockeyFilter]) -> Generator[str, None, None]: """ Get all units from a Juju status that match a list of filters. @@ -784,27 +744,20 @@ def filter_units( for unit in get_units(status): # Check unit filters - if not all( - check_filter_match(u_filter, unit) for u_filter in unit_filters - ): + if not all(check_filter_match(u_filter, unit) for u_filter in unit_filters): continue if app_filters or charm_filters: # Check application filters app = unit_to_application(status, unit) assert app - if not all( - check_filter_match(a_filter, app) for a_filter in app_filters - ): + if not all(check_filter_match(a_filter, app) for a_filter in app_filters): continue # Check charm filters charm = application_to_charm(status, app) assert charm - if not all( - check_filter_match(c_filter, charm) - for c_filter in charm_filters - ): + if not all(check_filter_match(c_filter, charm) for c_filter in charm_filters): continue # If there aren't any machine, IP, or hostname filters, just yield @@ -815,36 +768,25 @@ def filter_units( # Check machine filters machine = unit_to_machine(status, unit) assert machine - if not all( - check_filter_match(m_filter, machine) - for m_filter in machine_filters - ): + if not all(check_filter_match(m_filter, machine) for m_filter in machine_filters): continue # Check hostname filters hostname = machine_to_hostname(status, machine) assert hostname - if not all( - check_filter_match(h_filter, hostname) - for h_filter in hostname_filters - ): + if not all(check_filter_match(h_filter, hostname) for h_filter in hostname_filters): continue # Check IP filters ips = machine_to_ips(status, machine) assert ips - if not all( - any(check_filter_match(i_filter, ip) for ip in ips) - for i_filter in ip_filters - ): + if not all(any(check_filter_match(i_filter, ip) for ip in ips) for i_filter in ip_filters): continue yield unit -def filter_machines( - status: JujuStatus, filters: List[JockeyFilter] -) -> Generator[str, None, None]: +def filter_machines(status: JujuStatus, filters: List[JockeyFilter]) -> Generator[str, None, None]: """ Get all machines from a Juju status that match a list of filters. @@ -871,19 +813,13 @@ def filter_machines( for machine in get_machines(status): # Check machine filters - if not all( - check_filter_match(m_filter, machine) - for m_filter in machine_filters - ): + if not all(check_filter_match(m_filter, machine) for m_filter in machine_filters): continue # Check hostname filters hostname = machine_to_hostname(status, machine) assert hostname - if not all( - check_filter_match(h_filter, hostname) - for h_filter in hostname_filters - ): + if not all(check_filter_match(h_filter, hostname) for h_filter in hostname_filters): continue # Check IP filters @@ -922,11 +858,7 @@ def main(args: argparse.Namespace): cache_juju_status() # Get status - status = ( - retrieve_juju_cache() - if not args.file - else read_local_juju_status_file(args.file) - ) + status = retrieve_juju_cache() if not args.file else read_local_juju_status_file(args.file) RETRIEVAL_MAP = { ObjectType.CHARM: None, @@ -947,16 +879,11 @@ def main(args: argparse.Namespace): if __name__ == "__main__": parser = argparse.ArgumentParser( - description=( - "Jockey - A Juju query language to put all your " - "Juju objects at your fingertips." - ) + description=("Jockey - A Juju query language to put all your " "Juju objects at your fingertips.") ) # Add cache refresh flag - parser.add_argument( - "--refresh", action="store_true", help="Force a cache update" - ) + parser.add_argument("--refresh", action="store_true", help="Force a cache update") # Add object type argument parser.add_argument( ```
johnlettman commented 3 weeks ago

Lastly, if you go in this direction, I'll happily follow up with baking a full matching workflow.

johnlettman commented 3 weeks ago

I've added a commit for single-file static executable bundling. The commit message explains it a bit:


This commit adds a simple configuration for Pyinstaller to Poetry using poetry-pyinstaller-plugin. It results in poetry build emitting a static binary target, easy for carrying along into restrictive environments without concern for the Python version or existence of dependencies.

Building:

poetry build

[...]
Preparing PyInstaller 6.10.0 environment /home/jlettman/.cache/pypoetry/virtualenvs/jockey-Msi6Fc7k-py3.10
Building binaries with PyInstaller Python 3.10 [manylinux_2_35_x86_64]
  - Building jockey SINGLE_FILE
[...]

Testing the binary:

dist/pyinstaller/manylinux_2_35_x86_64/jockey --help

usage: jockey [-h] [--refresh] [-f FILE] object [filters ...]

Jockey - A Juju query language to put all your Juju objects at your fingertips.
[...]

Checking the output:

file dist/pyinstaller/manylinux_2_35_x86_64/jockey

dist/pyinstaller/manylinux_2_35_x86_64/jockey: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=04804d3c31218f938502cbed5cdd1af09d59a8f0, for GNU/Linux 2.6.32, stripped
johnlettman commented 3 weeks ago

This should also close #27

johnlettman commented 3 weeks ago

New workflow

image

It will attach the dist binaries for each OS to the workflow.

johnlettman commented 3 weeks ago

Closes #34 if CODECOV_TOKEN is added to the repository secrets.

johnlettman commented 3 weeks ago

Added automatic documentation workflow using pdoc: image

LCVcode commented 3 weeks ago

Wow, this is an enormous PR. Very nice.

I have some mixed feelings about the 120 character width linting. I happen to use a portrait monitor, so I like that narrower code look. But, I just tested it and I am able to fit 120 character lines without any issue, so I am okay with setting it as the convention for this project.

Let's modify that black lint I have in CI to use black -l 120 then get this huge PR merged.

johnlettman commented 3 weeks ago

Sorry about the enormity, haha. I aim to get the development environment in a nice place to help move away from the single-file Python file to a portable executable. Then, of course, there are the pipelines and documentation sites.

Let's modify that black lint I have in CI to use black -l 120 then get this huge PR merged.

Would you like this as a separate PR for your existing ci.yml? This is currently implemented in the PR: https://github.com/LCVcode/jockey/blob/a6f6eae4f0dc98c93a69c085d9f909c1b9eaf301/pyproject.toml#L56-L57

https://github.com/LCVcode/jockey/blob/a6f6eae4f0dc98c93a69c085d9f909c1b9eaf301/.github/workflows/ci.yml#L62-L88

johnlettman commented 3 weeks ago

Closes #34 if CODECOV_TOKEN is added to the repository secrets.

Also, @LCVcode, as a note here: the new workflow(s) support automatically detects if CodeCov is enabled by checking for the CODECOV_TOKEN repository action secret. It's free for FOSS repositories, such as this one, so it may be a helpful tool to track test coverage per #34.

johnlettman commented 3 weeks ago

Added snap capability per #7: image

johnlettman commented 3 weeks ago

Sorry for the noise, the snapcraft action seems to act vastly different than the local version.

johnlettman commented 3 weeks ago

SUCCESS! The snaps are built correctly now, and they're in the workflow. Phew!

johnlettman commented 3 weeks ago

New workflow: image

Artifacts: image

LCVcode commented 3 weeks ago

You're a beast! Thank you for all the crazy contributions.

johnlettman commented 3 weeks ago

No problem! :D I'm happy to help (plus, I did say I'd contribute back at Canonical)