fix: cancel_task returns 404 after successful cancellation and KeyError for forum in search#659
Open
octo-patch wants to merge 1 commit into
Open
fix: cancel_task returns 404 after successful cancellation and KeyError for forum in search#659octo-patch wants to merge 1 commit into
octo-patch wants to merge 1 commit into
Conversation
… missing forum key in api_ports Fixes 666ghj#630 Two bugs are fixed: 1. cancel_task in ReportEngine/flask_interface.py: When the current running task was cancelled, its status was updated to 'cancelled' before the registry lookup checked `task.status == 'running'`, causing the check to fail and the endpoint to return 404 even though cancellation succeeded. Fixed by tracking whether cancellation already happened via a `cancelled` flag before falling through to the registry check. 2. app.py search endpoint: `api_ports` lacked a 'forum' key but `running_apps` can include 'forum', causing a KeyError crash. Fixed by using `.get()` and skipping apps without a searchable API port (forum is a background monitor with no web API).
dosubot
Bot
added
the
size:L
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #630
Problem
Two bugs were reported in issue #630:
1.
cancel_taskalways returns 404 after cancelling the current running taskWhen
current_taskmatched the giventask_idand its status was"running":current_task.update_status("cancelled", ...)changed the status to"cancelled".current_task = Nonecleared the reference.task = tasks_registry.get(task_id)retrieved the same object from the registry.if task and task.status == 'running'— failed, because status is now"cancelled".elsebranch and returned 404, even though cancellation succeeded.2.
KeyErrorcrash in the search endpoint whenforumis runningapi_ports = {'insight': 8501, 'media': 8502, 'query': 8503}has no'forum'key, butrunning_appsis built from allprocessesentries (which includes'forum'). The direct dict accessapi_ports[app_name]raisedKeyErrorwhenforumwas running.Solution
Bug 1: Introduce a
cancelledflag. Set it toTrueimmediately after cancellingcurrent_task, and check it before falling through to the registry lookup. The registry check (task.status == 'running') is only performed whencurrent_taskwas not responsible for the cancellation.Bug 2: Replace
api_ports[app_name]withapi_ports.get(app_name). If the port isNone(e.g. forforum, which has no searchable web API), skip that app withcontinue.Testing
Added
tests/test_cancel_task.pywith 5 tests that verify:current_tasknow returns 200 (regression test for the main bug)current_taskis cleared from the global but not re-cancelled