Skip to content

Commit

Permalink
fix: change force flag semantics
Browse files Browse the repository at this point in the history
skip all hooks execution → skip version hook before installation
  • Loading branch information
livioso committed Oct 14, 2020
1 parent bac32f1 commit 8d46a73
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 68 deletions.
7 changes: 2 additions & 5 deletions src/upparat/statemachine/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,13 @@ def start_download_thread(self):

def on_enter(self, state, event):
hook = settings.hooks.download
force = self.job.force

if hook and not force:
if hook:
self.stop_download_hook = run_hook(
hook, self.root_machine.inbox, args=[self.job.meta]
)
else:
logger.info(
f"Skip download hook: Hook={hook if hook else 'no-hook'}, force={force}."
)
logger.info("Skip download hook: no-hook defined.")
self.start_download_thread()

def stop_hooks(self):
Expand Down
4 changes: 1 addition & 3 deletions src/upparat/statemachine/restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ def on_enter(self, state, event):
logger.info("Initiate restart")
self.job_progress(JobProgressStatus.REBOOT_START.value)
self.stop_restart_hook = run_hook(
settings.hooks.restart,
self.root_machine.inbox,
args=[self.job.meta, self.job.force],
settings.hooks.restart, self.root_machine.inbox, args=[self.job.meta]
)
else:
logger.info("No restart hook provided")
Expand Down
2 changes: 1 addition & 1 deletion src/upparat/statemachine/verify_installation.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def __init__(self):
super().__init__()

def on_enter(self, state, event):
if self.job.force or not settings.hooks.version:
if not settings.hooks.version:
logger.info("Skip version check")
self.job_succeeded(JobSuccessStatus.COMPLETE_NO_VERSION_CHECK.value)
self.publish(pysm.Event(JOB_INSTALLATION_COMPLETE))
Expand Down
13 changes: 9 additions & 4 deletions src/upparat/statemachine/verify_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,18 @@ def __init__(self):

def on_enter(self, state, event):
if self.job.status == JobStatus.QUEUED.value:
if self.job.force or not settings.hooks.version:
logger.info("Skip version check")
version_hook = settings.hooks.version
force = self.job.force

if force or not version_hook:
logger.info(
f"Skip version check [force={force}, {version_hook if version_hook else 'no-hook'}]" # noqa
)
return self._job_verified()

logger.debug("Start version check")
# Start version check
self.stop_version_hook = run_hook(
settings.hooks.version, self.root_machine.inbox, args=[self.job.meta]
version_hook, self.root_machine.inbox, args=[self.job.meta]
)

elif self.job.status == JobStatus.IN_PROGRESS.value:
Expand Down
28 changes: 3 additions & 25 deletions tests/statemachine/download_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,15 +265,17 @@ def test_on_job_cancelled(mocker, download_state):
assert state.stop_download_hook.is_set()


@pytest.mark.parametrize("force", [True, False])
def test_download_hook_completed(
mocker, download_state, urllib_urlopen_mock, create_hook_event
mocker, download_state, force, urllib_urlopen_mock, create_hook_event
):
urlopen_side_effect = [b"_", b""]
urlopen_mock = urllib_urlopen_mock(side_effect=urlopen_side_effect)
mocker.patch("urllib.request.urlopen", urlopen_mock)

state, inbox, _, _, run_hook = download_state
settings.hooks.download = "./download.sh"
state.job.force = force

state.on_enter(None, None)
hook_event = create_hook_event(settings.hooks.download, HOOK_STATUS_COMPLETED)
Expand All @@ -291,30 +293,6 @@ def test_download_hook_completed(
assert fd.read() == "_"


def test_download_hook_with_force_flag_set(
mocker, download_state, urllib_urlopen_mock, create_hook_event
):
urlopen_side_effect = [b"_", b""]
urlopen_mock = urllib_urlopen_mock(side_effect=urlopen_side_effect)
mocker.patch("urllib.request.urlopen", urlopen_mock)

state, inbox, _, _, run_hook = download_state
# hook but force is also set → ignore hook
settings.hooks.download = "./download.sh"
state.job.force = True

state.on_enter(None, None)

# wait for download to complete
event = inbox.get(timeout=TIMEOUT)
assert event.name == DOWNLOAD_COMPLETED
assert run_hook.call_count == 0

# check downloaded file
with open(state.job.filepath, "r") as fd:
assert fd.read() == "_"


@pytest.mark.parametrize("failure", [HOOK_STATUS_FAILED, HOOK_STATUS_TIMED_OUT])
def test_download_hook_failures(
mocker, download_state, failure, urllib_urlopen_mock, create_hook_event
Expand Down
4 changes: 1 addition & 3 deletions tests/statemachine/restart_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ def test_on_enter_restart_hook(restart_state):
),
)

run_hook.assert_called_once_with(
settings.hooks.restart, inbox, args=[JOB_.meta, JOB_.force]
)
run_hook.assert_called_once_with(settings.hooks.restart, inbox, args=[JOB_.meta])


def test_on_job_cancelled(restart_state):
Expand Down
30 changes: 3 additions & 27 deletions tests/statemachine/verify_installation_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,36 +76,12 @@ def test_on_enter_no_hook(verify_installation_state):
)


def test_on_enter_force_job(verify_installation_state):
state, inbox, mqtt_client, _, _ = verify_installation_state

settings.hooks.version = "./version.sh"
state.job = create_job_with(force=True)

state.on_enter(None, None)

assert inbox.qsize() == 1
published_event = inbox.get_nowait()
assert published_event.name == JOB_INSTALLATION_COMPLETE

mqtt_client.publish.assert_called_once_with(
f"$aws/things/{settings.broker.thing_name}/jobs/{state.job.id_}/update",
json.dumps(
{
"status": JobStatus.SUCCEEDED.value,
"statusDetails": {
"state": JobSuccessStatus.COMPLETE_NO_VERSION_CHECK.value,
"message": "none",
},
}
),
)


def test_on_enter_hook_executed(verify_installation_state):
@pytest.mark.parametrize("force", [True, False])
def test_on_enter_hook_executed(verify_installation_state, force):
state, inbox, _, _, run_hook = verify_installation_state

settings.hooks.version = "./version.sh"
state.job = create_job_with(force=force)

state.on_enter(None, None)

Expand Down

0 comments on commit 8d46a73

Please sign in to comment.