Skip to content

Commit

Permalink
prefer sys.exit over Abort
Browse files Browse the repository at this point in the history
  • Loading branch information
CamDavidsonPilon committed Dec 14, 2024
1 parent 791c6dc commit 0cdf51c
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 26 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
### Upcoming
- API to set clock time
- deprecated `default` in background_jobs yaml fields.
- [ ] test self-test

### 24.12.10
- Hotfix for UI settings bug

Expand Down
5 changes: 3 additions & 2 deletions pioreactor/actions/leader/export_experiment_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# See create_tables.sql for all tables
from __future__ import annotations

import sys
from base64 import b64decode
from contextlib import closing
from contextlib import ExitStack
Expand Down Expand Up @@ -137,11 +138,11 @@ def export_experiment_data(

if not output.endswith(".zip"):
click.echo("output should end with .zip")
raise click.Abort()
sys.exit(1)

if len(dataset_names) == 0:
click.echo("At least one dataset name must be provided.")
raise click.Abort()
sys.exit(1)

logger = create_logger("export_experiment_data")
logger.info(
Expand Down
4 changes: 2 additions & 2 deletions pioreactor/background_jobs/od_reading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1214,11 +1214,11 @@ def create_channel_angle_map(
def start_od_reading(
od_angle_channel1: Optional[pt.PdAngleOrREF] = None,
od_angle_channel2: Optional[pt.PdAngleOrREF] = None,
interval: Optional[float] = 1 / config.getfloat("od_reading.config", "samples_per_second"),
interval: Optional[float] = 1 / config.getfloat("od_reading.config", "samples_per_second", fallback=0.2),
fake_data: bool = False,
unit: Optional[str] = None,
experiment: Optional[str] = None,
use_calibration: bool = config.getboolean("od_reading.config", "use_calibration"),
use_calibration: bool = config.getboolean("od_reading.config", "use_calibration", fallback="True"),
) -> ODReader:
"""
This function prepares ODReader and other necessary transformation objects. It's a higher level API than using ODReader.
Expand Down
9 changes: 5 additions & 4 deletions pioreactor/cli/pio.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import annotations

import subprocess
import sys
from shlex import quote
from typing import Optional

Expand Down Expand Up @@ -427,7 +428,7 @@ def update_app(
commands_and_priority.append((f"sudo pip3 install --force-reinstall --no-index {source}", 1))
else:
click.echo("Not a valid source file. Should be either a whl or release archive.")
raise click.Abort()
sys.exit(1)

elif branch is not None:
cleaned_branch = quote(branch)
Expand All @@ -447,7 +448,7 @@ def update_app(
response = get(f"https://api.github.com/repos/{repo}/releases/{tag}")
if not response.ok:
logger.error(f"Version {version} not found")
raise click.Abort()
sys.exit(1)

release_metadata = loads(response.body)
version_installed = release_metadata["tag_name"]
Expand Down Expand Up @@ -530,7 +531,7 @@ def update_app(
logger.debug(p.stderr)
logger.error("Update failed. See logs.")
# end early
raise click.Abort()
sys.exit(1)
elif p.stdout:
logger.debug(p.stdout)

Expand Down Expand Up @@ -594,7 +595,7 @@ def update_firmware(version: Optional[str]) -> None:
logger.debug(p.stderr)
logger.error("Update failed. See logs.")
# end early
raise click.Abort()
sys.exit(1)

logger.info(f"Updated Pioreactor firmware to version {version_installed}.") # type: ignore

Expand Down
35 changes: 18 additions & 17 deletions pioreactor/cli/pios.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
from __future__ import annotations

import sys
from concurrent.futures import ThreadPoolExecutor

import click
Expand Down Expand Up @@ -46,7 +47,7 @@ def pios(ctx) -> None:
# this is run even if workers run `pios plugins etc.`
if not am_I_leader():
click.echo("workers cannot run `pios` commands. Try `pio` instead.", err=True)
raise click.Abort()
sys.exit(1)


if am_I_leader() or is_testing_env():
Expand Down Expand Up @@ -189,7 +190,7 @@ def cp(
if not y:
confirm = input(f"Confirm copying {filepath} onto {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("cp", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)

Expand All @@ -207,7 +208,7 @@ def _thread_function(unit: str) -> bool:
results = executor.map(_thread_function, units)

if not all(results):
raise click.Abort()
sys.exit(1)

@pios.command("rm", short_help="rm a file across the cluster")
@click.argument("filepath", type=click.Path(resolve_path=True))
Expand All @@ -223,7 +224,7 @@ def rm(
if not y:
confirm = input(f"Confirm deleting {filepath} on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("rm", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)

Expand All @@ -244,7 +245,7 @@ def _thread_function(unit: str) -> bool:
results = executor.map(_thread_function, units)

if not all(results):
raise click.Abort()
sys.exit(1)

@pios.group(invoke_without_command=True)
@click.option("-s", "--source", help="use a release-***.zip already on the workers")
Expand All @@ -267,7 +268,7 @@ def update(
if not y:
confirm = input(f"Confirm updating app and ui on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("update", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)
options: dict[str, str | None] = {}
Expand Down Expand Up @@ -329,7 +330,7 @@ def update_app(
if not y:
confirm = input(f"Confirm updating app on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("update", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)
options: dict[str, str | None] = {}
Expand Down Expand Up @@ -397,7 +398,7 @@ def update_ui(
if not y:
confirm = input(f"Confirm updating ui on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("update", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)
options: dict[str, str | None] = {}
Expand Down Expand Up @@ -459,7 +460,7 @@ def install_plugin(plugin: str, source: str | None, units: tuple[str, ...], y: b
if not y:
confirm = input(f"Confirm installing {plugin} on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("install_plugin", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)
commands = {"args": [plugin], "options": {}}
Expand Down Expand Up @@ -504,7 +505,7 @@ def uninstall_plugin(plugin: str, units: tuple[str, ...], y: bool, json: bool) -
if not y:
confirm = input(f"Confirm uninstalling {plugin} on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

logger = create_logger("uninstall_plugin", unit=get_unit_name(), experiment=UNIVERSAL_EXPERIMENT)
commands = {"args": [plugin]}
Expand Down Expand Up @@ -587,7 +588,7 @@ def _thread_function(unit: str) -> bool:
results = executor.map(_thread_function, units)

if not all(results):
raise click.Abort()
sys.exit(1)

@pios.command("kill", short_help="kill a job(s) on workers")
@click.option("--job")
Expand Down Expand Up @@ -626,7 +627,7 @@ def kill(
if not y:
confirm = input(f"Confirm killing jobs on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

with ClusterJobManager() as cm:
results = cm.kill_jobs(
Expand Down Expand Up @@ -672,15 +673,15 @@ def run(ctx, job: str, units: tuple[str, ...], y: bool, json: bool) -> None:

if "unit" in extra_args:
click.echo("Did you mean to use 'units' instead of 'unit'? Exiting.", err=True)
raise click.Abort()
sys.exit(1)

units = universal_identifier_to_all_active_workers(units)
assert len(units) > 0, "Empty units!"

if not y:
confirm = input(f"Confirm running {job} on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

data = parse_click_arguments(extra_args)

Expand Down Expand Up @@ -722,7 +723,7 @@ def shutdown(units: tuple[str, ...], y: bool) -> None:
if not y:
confirm = input(f"Confirm shutting down on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

def _thread_function(unit: str) -> bool:
try:
Expand Down Expand Up @@ -755,7 +756,7 @@ def reboot(units: tuple[str, ...], y: bool) -> None:
if not y:
confirm = input(f"Confirm rebooting on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

def _thread_function(unit: str) -> bool:
try:
Expand Down Expand Up @@ -800,7 +801,7 @@ def update_settings(ctx, job: str, units: tuple[str, ...], y: bool) -> None:
if not y:
confirm = input(f"Confirm updating {job}'s {extra_args} on {units}? Y/n: ").strip()
if confirm != "Y":
raise click.Abort()
sys.exit(1)

units = universal_identifier_to_all_active_workers(units)

Expand Down
2 changes: 1 addition & 1 deletion pioreactor/cluster_management/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def add_worker(hostname: str, password: str, version: str, model: str) -> None:
logger.error(
f"`{hostname}` not found on network after {round(elapsed())} seconds. Check that you provided the right i) the name is correct, ii) worker is powered on, iii) any WiFi credentials to the network are correct."
)
sys.exit()
sys.exit(1)
sleep(sleep_time)

res = subprocess.run(
Expand Down

0 comments on commit 0cdf51c

Please sign in to comment.