Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(188)

Issue 6155422901731328: Run Rietveld using the AppEngine SDK (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by Wladimir Palant
Modified:
4 years, 4 months ago
Visibility:
Public.

Description

I suspect that upload.py will still fail to authenticate - but other than that it seems to be working.

Patch Set 1 #

Total comments: 15

Patch Set 2 : Addressed comments and added proper configuration #

Patch Set 3 : Added missing file and a comment #

Total comments: 1

Patch Set 4 : Enabled OAuth verification as well #

Total comments: 1

Patch Set 5 : Addressed comments and added caching #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+311 lines, -326 lines) Patch
M modules/codereview/manifests/init.pp View 1 2 3 4 2 chunks +1 line, -48 lines 0 comments Download
M modules/customservice/manifests/init.pp View 1 chunk +2 lines, -2 lines 0 comments Download
M modules/customservice/templates/init-customservice.erb View 0 chunks +-1 lines, --1 lines 0 comments Download
M modules/discourse/manifests/init.pp View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
R modules/private-stub/files/rietveld-auth-users.json View 1 1 chunk +0 lines, -21 lines 0 comments Download
M modules/private-stub/hiera/roles/codereviewserver.yaml View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
R modules/rietveld/files/Makefile View 1 chunk +0 lines, -46 lines 0 comments Download
R modules/rietveld/files/rietveld.conf View 1 chunk +0 lines, -14 lines 0 comments Download
M modules/rietveld/files/site.conf View 1 chunk +5 lines, -1 line 0 comments Download
A modules/rietveld/files/wrapper.py View 1 2 3 4 1 chunk +227 lines, -0 lines 2 comments Download
M modules/rietveld/manifests/init.pp View 1 2 3 4 1 chunk +63 lines, -75 lines 0 comments Download
A modules/rietveld/templates/config.ini.erb View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
R modules/rietveld/templates/settings.py.erb View 1 chunk +0 lines, -113 lines 0 comments Download

Messages

Total messages: 13
Wladimir Palant
4 years, 4 months ago (2015-06-02 21:00:02 UTC) #1
Wladimir Palant
Note that this is still work in progress, even though OAuth works now. OAuth credentials ...
4 years, 4 months ago (2015-06-02 21:19:18 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py File modules/rietveld/files/wrapper.py (right): http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py#newcode17 modules/rietveld/files/wrapper.py:17: sys.path[0:0] = paths.script_paths(script_name) Nit: sys.path.insert(0, ..) http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py#newcode48 modules/rietveld/files/wrapper.py:48: def ...
4 years, 4 months ago (2015-06-02 21:32:05 UTC) #3
Sebastian Noack
http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py File modules/rietveld/files/wrapper.py (right): http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py#newcode109 modules/rietveld/files/wrapper.py:109: data = urllib.urlopen('https://accounts.google.com/o/oauth2/token', urllib.urlencode(token_params)).read() Please close the file-like object ...
4 years, 4 months ago (2015-06-02 21:41:06 UTC) #4
Wladimir Palant
I replaced cookie_secret by a config.ini file - OAuth2 credentials and admins are being configured ...
4 years, 4 months ago (2015-06-03 15:42:35 UTC) #5
Sebastian Noack
LGTM on the Python code. I leave reviewing the Puppet scripts up to Matze. http://codereview.adblockplus.org/6155422901731328/diff/5629499534213120/modules/rietveld/files/wrapper.py ...
4 years, 4 months ago (2015-06-03 15:58:49 UTC) #6
Wladimir Palant
Overriding OAuth validation as well now - Rietveld is using it even though they have ...
4 years, 4 months ago (2015-06-03 18:28:04 UTC) #7
mathias
http://codereview.adblockplus.org/6155422901731328/diff/5194384987389952/modules/discourse/manifests/init.pp File modules/discourse/manifests/init.pp (right): http://codereview.adblockplus.org/6155422901731328/diff/5194384987389952/modules/discourse/manifests/init.pp#newcode324 modules/discourse/manifests/init.pp:324: Customservice <| |> { This will implicitly realize all ...
4 years, 4 months ago (2015-06-03 18:33:07 UTC) #8
Wladimir Palant
I've addressed Matze's comments and added user info caching so that uploading base files is ...
4 years, 4 months ago (2015-06-04 21:23:58 UTC) #9
Sebastian Noack
Still LGTM on the Python code. http://codereview.adblockplus.org/6155422901731328/diff/5631943370604544/modules/rietveld/files/wrapper.py File modules/rietveld/files/wrapper.py (right): http://codereview.adblockplus.org/6155422901731328/diff/5631943370604544/modules/rietveld/files/wrapper.py#newcode195 modules/rietveld/files/wrapper.py:195: user_id = '1' ...
4 years, 4 months ago (2015-06-04 21:51:43 UTC) #10
mathias
LGTM.
4 years, 4 months ago (2015-06-04 22:19:34 UTC) #11
Wladimir Palant
http://codereview.adblockplus.org/6155422901731328/diff/5631943370604544/modules/rietveld/files/wrapper.py File modules/rietveld/files/wrapper.py (right): http://codereview.adblockplus.org/6155422901731328/diff/5631943370604544/modules/rietveld/files/wrapper.py#newcode195 modules/rietveld/files/wrapper.py:195: user_id = '1' + ''.join(['%02d' % ord(x) for x ...
4 years, 4 months ago (2015-06-04 23:19:54 UTC) #12
Wladimir Palant
4 years, 4 months ago (2015-06-04 23:23:52 UTC) #13
Pushed: https://hg.adblockplus.org/infrastructure/rev/2f9490cd2f9e

I guess Matze will reimage the server new.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5