hgreview.py - Issue 29338096: Issue 3783 - Redirect auth page to review after upload

Side by Side Diff: hgreview.py

Issue 29338096: Issue 3783 - Redirect auth page to review after upload (Closed)

Patch Set: Don't inherit globals Created March 10, 2016, 2:50 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
	1 import BaseHTTPServer

1 import os	2 import os

	3 import random

2 import re	4 import re

3 import subprocess

4 import sys	5 import sys

5 import urllib	6 import urllib

6	7

7 from mercurial import cmdutil, error	8 from mercurial import cmdutil, error

8	9

	10 SERVER = 'https://codereview.adblockplus.org'

	11 UPLOADTOOL_URL = SERVER + '/static/upload.py'

	12

9 cmdtable = {}	13 cmdtable = {}

10 command = cmdutil.command(cmdtable)	14 command = cmdutil.command(cmdtable)

11	15

12 @command('review',	16 @command('review',

13 [	17 [

14 ('i', 'issue', '', 'If given, adds a patch set to this review, otherwise cre ate a new one.', 'ISSUE'),	18 ('i', 'issue', '', 'If given, adds a patch set to this review, otherwise cre ate a new one.', 'ISSUE'),

15 ('r', 'revision', '', 'Revision to diff against or a revision range to uploa d.', 'REV'),	19 ('r', 'revision', '', 'Revision to diff against or a revision range to uploa d.', 'REV'),

16 ('c', 'change', '', 'A single revision to upload.', 'REV'),	20 ('c', 'change', '', 'A single revision to upload.', 'REV'),

17 ('t', 'title', '', 'New review subject or new patch set title.', 'TITLE'),	21 ('t', 'title', '', 'New review subject or new patch set title.', 'TITLE'),

18 ('m', 'message', '', 'New review description or new patch set message.', 'ME SSAGE'),	22 ('m', 'message', '', 'New review description or new patch set message.', 'ME SSAGE'),

19 ('w', 'reviewers', '', 'Add reviewers (comma separated email addresses or @a dblockplus.org user names).', 'REVIEWERS'),	23 ('w', 'reviewers', '', 'Add reviewers (comma separated email addresses or @a dblockplus.org user names).', 'REVIEWERS'),

20 ('', 'cc', '', 'Add CC (comma separated email addresses or @adblockplus.org user names).', 'CC'),	24 ('', 'cc', '', 'Add CC (comma separated email addresses or @adblockplus.org user names).', 'CC'),

21 ('', 'private', None, 'Make the review restricted to reviewers and those CCe d.'),	25 ('', 'private', None, 'Make the review restricted to reviewers and those CCe d.'),

22 ('y', 'assume_yes', None, 'Assume that the answer to yes/no questions is \'y es\'.'),	26 ('y', 'assume_yes', None, 'Assume that the answer to yes/no questions is \'y es\'.'),

23 ('', 'print_diffs', None, 'Print full diffs.'),	27 ('', 'print_diffs', None, 'Print full diffs.'),

24 ], '[options] [path...]')	28 ], '[options] [path...]')

25 def review(ui, repo, paths, *opts):	29 def review(ui, repo, paths, *opts):

26 '''	30 '''

27 Uploads a review to https://codereview.adblockplus.org/ or updates an	31 Uploads a review to https://codereview.adblockplus.org/ or updates an

28 existing review request. This will always send mails for new reviews, when	32 existing review request. This will always send mails for new reviews, when

29 updating a review mails will only be sent if a message is given.	33 updating a review mails will only be sent if a message is given.

30 '''	34 '''

31 args = ['--oauth2']	35 args = ['--oauth2', '--server', SERVER]

32 if ui.debugflag:	36 if ui.debugflag:

33 args.append('--noisy')	37 args.append('--noisy')

34 elif ui.verbose:	38 elif ui.verbose:

35 args.append('--verbose')	39 args.append('--verbose')

36 elif ui.quiet:	40 elif ui.quiet:

37 args.append('--quiet')	41 args.append('--quiet')

38	42

39 if not opts.get('issue') or opts.get('message'):	43 if not opts.get('issue') or opts.get('message'):

40 args.append('--send_mail')	44 args.append('--send_mail')

41	45

(...skipping 24 matching lines...) Expand all Loading...
66 for opt in ('private', 'assume_yes', 'print_diffs'):	70 for opt in ('private', 'assume_yes', 'print_diffs'):

67 if opts.get(opt, False):	71 if opts.get(opt, False):

68 args.append('--' + opt)	72 args.append('--' + opt)

69	73

70 args.extend(paths)	74 args.extend(paths)

71	75

72 upload_path = ui.config('review', 'uploadtool_path',	76 upload_path = ui.config('review', 'uploadtool_path',

73 os.path.join('~', '.hgreview_upload.py'))	77 os.path.join('~', '.hgreview_upload.py'))

74 upload_path = os.path.expanduser(upload_path)	78 upload_path = os.path.expanduser(upload_path)

75 if not os.path.exists(upload_path):	79 if not os.path.exists(upload_path):

76 url = 'https://codereview.adblockplus.org/static/upload.py'	80 ui.status('Downloading {0} to {1}.\n'.format(UPLOADTOOL_URL, upload_path))

77 ui.status('Downloading {0} to {1}.\n'.format(url, upload_path))	81 urllib.urlretrieve(UPLOADTOOL_URL, upload_path)

78 urllib.urlretrieve(url, upload_path)

79	82

80 subprocess.call([sys.executable, upload_path] + args)	83 # Modify upload tool's auth response in order to redirect to the issue

	84 port = random.randrange(2000, 60000)
	Sebastian Noack 2016/03/10 15:08:22 I'm not sure whether it's a good idea to use a ran I'm not sure whether it's a good idea to use a random port, at least not without fallback logic if that port is already used. What if a different server, or an instance of this script, is already running on the port we randomly choosed? I think we should rather handle the error when the port is already in use and fall back to another port. Whether the next port is picked incrementally or randomly shouldn't matter much. Wladimir Palant 2016/03/11 10:12:42 Ok, now we will try ten different ports. In the un Show quoted text On 2016/03/10 15:08:22, Sebastian Noack wrote: > I'm not sure whether it's a good idea to use a random port, at least not without > fallback logic if that port is already used. Ok, now we will try ten different ports. In the unlikely event that none of them work we'll simply fall back to the original upload.py logic.
	85 scope = {}

	86 execfile(upload_path, scope)

	87 scope['AUTH_HANDLER_RESPONSE'] = '''\

	88 <html>

	89 <head>

	90 <title>Authentication Status</title>

	91 <script>

	92 window.onload = function()

	93 {

	94 setInterval(function()

	95 {

	96 var script = document.createElement("script");

	97 script.src = "http://localhost:%s/?" + (new Date().getTime());

	98 document.body.appendChild(script);

	99 }, 1000)

	100 }

	101 </script>

	102 </head>

	103 <body>

	104 <p>

	105 The authentication flow has completed. This page will redirect to your

	106 review shortly.

	107 </p>

	108 </body>

	109 </html>

	110 ''' % port

	111

	112 # Run the upload tool

	113 issue, patchset = scope['RealMain']([upload_path] + args)

	114

	115 # Wait for the page to check in and retrieve issue URL

	116 class RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):

	117 def do_GET(self):

	118 self.send_response(200)

	119 self.send_header('Content-type', 'text/javascript')

	120 self.end_headers()

	121 self.wfile.write('location.href = "{0}";'.format(SERVER + '/' + issue))

	122 def log_message(args, *kwargs):

	123 pass

	124

	125 server = BaseHTTPServer.HTTPServer(('localhost', port), RequestHandler)

	126 server.handle_request()

OLD	NEW

« no previous file with comments | « no previous file | no next file » | no next file with comments »