From c126db811e3de025c0c780c150ffc8d1c0c6d77d Mon Sep 17 00:00:00 2001 From: Phil Hagen Date: Sat, 19 Oct 2019 11:01:12 +0800 Subject: [PATCH 1/2] add async/background hook script capability --- README.rst | 12 ++++++++++ webhooks.py | 63 ++++++++++++++++++++++++++++++++++++----------------- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/README.rst b/README.rst index b070e99..7c1bb4c 100644 --- a/README.rst +++ b/README.rst @@ -62,11 +62,18 @@ following order: :: + hooks/{event}-{name}-{branch}-background hooks/{event}-{name}-{branch} + hooks/{event}-{name}-background hooks/{event}-{name} + hooks/{event}-background hooks/{event} + hooks/all-background hooks/all +Hook files with the "-background" suffix will be spawned asyncronously, and +STDOUT/STDERR and return code will not be captured. + The application will pass to the hooks the path to a JSON file holding the payload for the request as first argument. The event type will be passed as second argument. For example: @@ -101,6 +108,11 @@ Not all events have an associated branch, so a branch-specific hook cannot fire for such events. For events that contain a pull_request object, the base branch (target for the pull request) is used, not the head branch. +Backgrounded hooks are responsible for cleaning up their own temp files, as +provided in argv[1]. The duplicate webhook content is required to preent a +race condition where the content file is deleted before the child process +can read it. + The payload structure depends on the event type. Please review: https://developer.github.com/v3/activity/events/types/ diff --git a/webhooks.py b/webhooks.py index 082c236..dcede93 100644 --- a/webhooks.py +++ b/webhooks.py @@ -31,9 +31,11 @@ from ipaddress import ip_address, ip_network from flask import Flask, request, abort - application = Flask(__name__) +###pjh +#application.url_map.strict_slashes = False +###pjh @application.route('/', methods=['GET', 'POST']) def index(): @@ -153,10 +155,14 @@ def index(): scripts = [] if branch and name: scripts.append(join(hooks, '{event}-{name}-{branch}'.format(**meta))) + scripts.append(join(hooks, '{event}-{name}-{branch}-background'.format(**meta))) if name: scripts.append(join(hooks, '{event}-{name}'.format(**meta))) + scripts.append(join(hooks, '{event}-{name}-background'.format(**meta))) scripts.append(join(hooks, '{event}'.format(**meta))) + scripts.append(join(hooks, '{event}-background'.format(**meta))) scripts.append(join(hooks, 'all')) + scripts.append(join(hooks, 'all-background')) # Check permissions scripts = [s for s in scripts if isfile(s) and access(s, X_OK)] @@ -172,23 +178,41 @@ def index(): ran = {} for s in scripts: - proc = Popen( - [s, tmpfile, event], - stdout=PIPE, stderr=PIPE - ) - stdout, stderr = proc.communicate() - - ran[basename(s)] = { - 'returncode': proc.returncode, - 'stdout': stdout.decode('utf-8'), - 'stderr': stderr.decode('utf-8'), - } - - # Log errors if a hook failed - if proc.returncode != 0: - logging.error('{} : {} \n{}'.format( - s, proc.returncode, stderr - )) + if s.endswith('-background'): + # each backgrounded script gets its own tempfile + # in this case, the backgrounded script MUST clean up after this!!! + # the per-job tempfile will NOT be deleted here! + osfd2, tmpfile2 = mkstemp() + with fdopen(osfd2, 'w') as pf2: + pf2.write(dumps(payload)) + + proc = Popen( + [s, tmpfile2, event], + stdout=PIPE, stderr=PIPE + ) + + ran[basename(s)] = { + 'backgrounded': 'yes' + } + + else: + proc = Popen( + [s, tmpfile, event], + stdout=PIPE, stderr=PIPE + ) + stdout, stderr = proc.communicate() + + ran[basename(s)] = { + 'returncode': proc.returncode, + 'stdout': stdout.decode('utf-8'), + 'stderr': stderr.decode('utf-8'), + } + + # Log errors if a hook failed + if proc.returncode != 0: + logging.error('{} : {} \n{}'.format( + s, proc.returncode, stderr + )) # Remove temporal file remove(tmpfile) @@ -201,6 +225,5 @@ def index(): logging.info(output) return output - if __name__ == '__main__': - application.run(debug=True, host='0.0.0.0') + application.run(debug=True, host='0.0.0.0') \ No newline at end of file From 4a9d08eba09f56d55a1058cb0b8ba0bf8832b3f0 Mon Sep 17 00:00:00 2001 From: Phil Hagen Date: Sat, 19 Oct 2019 11:04:15 +0800 Subject: [PATCH 2/2] use strict_slashes to avoid 308 redirect --- webhooks.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/webhooks.py b/webhooks.py index dcede93..1b41fed 100644 --- a/webhooks.py +++ b/webhooks.py @@ -32,10 +32,7 @@ from flask import Flask, request, abort application = Flask(__name__) - -###pjh -#application.url_map.strict_slashes = False -###pjh +application.url_map.strict_slashes = False @application.route('/', methods=['GET', 'POST']) def index():