Skip to content

Commit

Permalink
split out logic to test directly
Browse files Browse the repository at this point in the history
  • Loading branch information
CamDavidsonPilon committed Sep 17, 2023
1 parent 7eded61 commit f2a67c9
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 20 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
file_glob: true

zip-asset-upload:
name: 🤐 Zip assets
runs-on: ubuntu-latest
needs: build
steps:
Expand Down
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
- SPI is on by default on all new image installs
- Plugin author information is presented on the `/plugins` page in the UI.


### 23.8.29
- Pioreactor's IPv4 and hostname is now displayed under System in the UI.
- In configuration, renamed section `dosing_automation` to `dosing_automation.config` (only applies to new installs). It's recommended for existing users to make this change, too.
Expand Down
45 changes: 27 additions & 18 deletions pioreactor/background_jobs/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from json import loads
from threading import Thread
from time import sleep
from typing import Any
from typing import Callable
from typing import Optional

Expand Down Expand Up @@ -587,8 +588,6 @@ def run_job_on_machine(self, msg: MQTTMessage) -> None:
# we use a thread here since we want to exit this callback without blocking it.
# a blocked callback can disconnect from MQTT broker, prevent other callbacks, etc.

from shlex import join # https://docs.python.org/3/library/shlex.html#shlex.quote

job_name = msg.topic.split("/")[-1]
payload = loads(msg.payload) if msg.payload else {"options": {}, "args": []}

Expand Down Expand Up @@ -634,22 +633,7 @@ def run_job_on_machine(self, msg: MQTTMessage) -> None:
Thread(target=pump_action, kwargs=options, daemon=True).start()

else:
prefix = "nohup"

core_command = ["pio", "run", job_name]

list_of_options: list[str] = []
for option, value in options.items():
list_of_options.append(f"--{option.replace('_', '-')}")
if value is not None:
# this handles flag arguments, like --dry-run
list_of_options.append(str(value))

suffix = ">/dev/null 2>&1 &"

# shell-escaped to protect against injection vulnerabilities, see join docs
# we don't escape the suffix.
command = prefix + " " + join(core_command + args + list_of_options) + " " + suffix
command = self._job_options_and_args_to_shell_command(job_name, args, options)

self.logger.debug(f"Running `{command}` from monitor job.")

Expand All @@ -660,6 +644,31 @@ def run_job_on_machine(self, msg: MQTTMessage) -> None:
daemon=True,
).start()

@staticmethod
def _job_options_and_args_to_shell_command(
job_name: str, args: list[str], options: dict[str, Any]
) -> str:
from shlex import join # https://docs.python.org/3/library/shlex.html#shlex.quote

prefix = "nohup"

core_command = ["pio", "run", job_name]

list_of_options: list[str] = []
for option, value in options.items():
list_of_options.append(f"--{option.replace('_', '-')}")
if value is not None:
# this handles flag arguments, like --dry-run
list_of_options.append(str(value))

suffix = ">/dev/null 2>&1 &"

# shell-escaped to protect against injection vulnerabilities, see join docs
# we don't escape the suffix.
command = prefix + " " + join(core_command + args + list_of_options) + " " + suffix

return command

def flicker_error_code_from_mqtt(self, message: MQTTMessage) -> None:
if self.led_in_use:
return
Expand Down
20 changes: 20 additions & 0 deletions pioreactor/tests/test_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,23 @@ def test_run_job_with_monitor() -> None:
pause()

assert any("pio run example_plugin" in msg["message"] for msg in bucket)


def test__job_options_and_args_to_shell_command() -> None:
m = Monitor
assert (
m._job_options_and_args_to_shell_command("stirring", [], {"target_rpm": 400})
== "nohup pio run stirring --target-rpm 400 >/dev/null 2>&1 &"
)
assert (
m._job_options_and_args_to_shell_command("stirring", [], {"ignore_rpm": None})
== "nohup pio run stirring --ignore-rpm >/dev/null 2>&1 &"
)
assert (
m._job_options_and_args_to_shell_command("stirring", [], {})
== "nohup pio run stirring >/dev/null 2>&1 &"
)
assert (
m._job_options_and_args_to_shell_command("od_calibration", ["list"], {})
== "nohup pio run od_calibration list >/dev/null 2>&1 &"
)
3 changes: 2 additions & 1 deletion update_scripts/upcoming/update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ set -e

export LC_ALL=C

wget -O /usr/local/bin/install_pioreactor_plugin.sh https://raw.githubusercontent.com/Pioreactor/CustoPiZer/a70a01c7f9736bbb43d126dbb1388722fe27b6a0/workspace/scripts/files/bash/install_pioreactor_plugin.sh
wget -O /usr/local/bin/install_pioreactor_plugin.sh https://raw.githubusercontent.com/Pioreactor/CustoPiZer/1d95fa4952398aab1dda09f8cc6d79a8e72197d4/workspace/scripts/files/bash/install_pioreactor_plugin.sh
sudo chown pioreactor:pioreactor /home/pioreactor/.pioreactor/experiment_profiles/demo_stirring_example.yaml || true

0 comments on commit f2a67c9

Please sign in to comment.