Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1176243002:
my_activity.py: update to use oauth for projecthosting (Closed)
|
Created:
5 years, 6 months ago by seanmccullough Modified:
5 years, 6 months ago CC:
chromium-reviews, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
Descriptionmy_activity.py: update to use oauth for projecthosting
BUG=491889
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=295626
Patch Set 1 #
Total comments: 16
Patch Set 2 : removed more dead/broken code, tidied up some hacks #Messages
Total messages: 17 (5 generated)
seanmccullough@chromium.org changed reviewers: + vadimsh@chromium.org
mostly looks good I was assuming the bug was about fixing my_activity.py completely. It has some more issues related to OAuth2 change other than code site one... I think it should be relatively easy to fix them too, left some pointers. https://codereview.chromium.org/1176243002/diff/1/auth.py File auth.py (right): https://codereview.chromium.org/1176243002/diff/1/auth.py#newcode223 auth.py:223: if hostname == 'https://code.google.com': make it a top-level config-like dict, so it looks less like a hack :) Also use parsed.netloc (will be "code.google.com") as a key, since that's what used in other places: ADDITIONAL_SCOPES = { 'code.google.com': 'https://www.googleapis.com/auth/projecthosting', } https://codereview.chromium.org/1176243002/diff/1/my_activity.py File my_activity.py (right): https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode106 my_activity.py:106: 'host': 'gerrit.chromium.org', this host has been turn off today, can be removed from this list https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode139 my_activity.py:139: def get_auth_token(email): this should be eventually deleted, it's broken https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode236 my_activity.py:236: cookie_file = os.path.expanduser('~/.codereview_upload_cookies') it's not directly related to OAuth code.google.com issue, but this code is broken too. ~/.codereview_upload_cookies no longer exists. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode251 my_activity.py:251: def has_cookie(instance): this can be reformulated as: a = auth.get_authenticator_for_host(<url>) return a.has_cached_credentials() https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode470 my_activity.py:470: "https://code.google.com", auth_config) just "code.google.com" here should work too https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode479 my_activity.py:479: 'q': '%s' % user_str, 'q': user_str https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode484 my_activity.py:484: _, body = http.request(url) this thing will raise auth.AuthenticationError if there's no cached token. Catch it somewhere in main() and ask user to login. Just printing the exception to stdout should be enough. It asks to run depot-tools-auth (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/auth.py#94).
Okay this appears to work when I run with -r (prints some crrev links) but I get this error: Looking up activity..... auth.AuthenticationError: Access to https://chromereviews.googleplex.com is denied (server returned HTTP 403). depot-tools-auth login codereviews.googleplex.com doesn't seem to help. https://codereview.chromium.org/1176243002/diff/1/auth.py File auth.py (right): https://codereview.chromium.org/1176243002/diff/1/auth.py#newcode223 auth.py:223: if hostname == 'https://code.google.com': On 2015/06/11 01:37:31, Vadim Sh. wrote: > make it a top-level config-like dict, so it looks less like a hack :) Also use > parsed.netloc (will be "code.google.com") as a key, since that's what used in > other places: > > ADDITIONAL_SCOPES = { > 'code.google.com': 'https://www.googleapis.com/auth/projecthosting', > } Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py File my_activity.py (right): https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode106 my_activity.py:106: 'host': 'gerrit.chromium.org', On 2015/06/11 01:37:31, Vadim Sh. wrote: > this host has been turn off today, can be removed from this list Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode139 my_activity.py:139: def get_auth_token(email): On 2015/06/11 01:37:31, Vadim Sh. wrote: > this should be eventually deleted, it's broken Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode236 my_activity.py:236: cookie_file = os.path.expanduser('~/.codereview_upload_cookies') On 2015/06/11 01:37:31, Vadim Sh. wrote: > it's not directly related to OAuth http://code.google.com issue, but this code is > broken too. ~/.codereview_upload_cookies no longer exists. removed it, now it just checks "has_cookie" https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode251 my_activity.py:251: def has_cookie(instance): On 2015/06/11 01:37:31, Vadim Sh. wrote: > this can be reformulated as: > > a = auth.get_authenticator_for_host(<url>) > return a.has_cached_credentials() Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode470 my_activity.py:470: "https://code.google.com", auth_config) On 2015/06/11 01:37:32, Vadim Sh. wrote: > just "code.google.com" here should work too Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode479 my_activity.py:479: 'q': '%s' % user_str, On 2015/06/11 01:37:31, Vadim Sh. wrote: > 'q': user_str Done. https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode484 my_activity.py:484: _, body = http.request(url) On 2015/06/11 01:37:31, Vadim Sh. wrote: > this thing will raise auth.AuthenticationError if there's no cached token. Catch > it somewhere in main() and ask user to login. Just printing the exception to > stdout should be enough. It asks to run depot-tools-auth > (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/auth.py#94). Done.
lgtm On 2015/06/11 02:05:23, seanmccullough wrote: > Okay this appears to work when I run with -r (prints some crrev links) but I get > this error: > > Looking up activity..... > auth.AuthenticationError: Access to https://chromereviews.googleplex.com is > denied (server returned HTTP 403). > > depot-tools-auth login http://codereviews.googleplex.com doesn't seem to help. > Most probably API endpoint that is used by rietveld_search is not whitelisted for OAuth access on chromereviews.googleplex.com. See (and modify if appropriate) https://chrome-internal.googlesource.com/infra/infra_internal/+/master/appeng... > https://codereview.chromium.org/1176243002/diff/1/auth.py > File auth.py (right): > > https://codereview.chromium.org/1176243002/diff/1/auth.py#newcode223 > auth.py:223: if hostname == 'https://code.google.com': > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > make it a top-level config-like dict, so it looks less like a hack :) Also use > > parsed.netloc (will be "code.google.com") as a key, since that's what used in > > other places: > > > > ADDITIONAL_SCOPES = { > > 'code.google.com': 'https://www.googleapis.com/auth/projecthosting', > > } > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py > File my_activity.py (right): > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode106 > my_activity.py:106: 'host': 'gerrit.chromium.org', > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > this host has been turn off today, can be removed from this list > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode139 > my_activity.py:139: def get_auth_token(email): > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > this should be eventually deleted, it's broken > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode236 > my_activity.py:236: cookie_file = > os.path.expanduser('~/.codereview_upload_cookies') > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > it's not directly related to OAuth http://code.google.com issue, but this code > is > > broken too. ~/.codereview_upload_cookies no longer exists. > > removed it, now it just checks "has_cookie" > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode251 > my_activity.py:251: def has_cookie(instance): > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > this can be reformulated as: > > > > a = auth.get_authenticator_for_host(<url>) > > return a.has_cached_credentials() > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode470 > my_activity.py:470: "https://code.google.com", auth_config) > On 2015/06/11 01:37:32, Vadim Sh. wrote: > > just "code.google.com" here should work too > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode479 > my_activity.py:479: 'q': '%s' % user_str, > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > 'q': user_str > > Done. > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode484 > my_activity.py:484: _, body = http.request(url) > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > this thing will raise auth.AuthenticationError if there's no cached token. > Catch > > it somewhere in main() and ask user to login. Just printing the exception to > > stdout should be enough. It asks to run depot-tools-auth > > > (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/auth.py#94). > > Done.
On 2015/06/11 02:20:58, Vadim Sh. wrote: > lgtm > > On 2015/06/11 02:05:23, seanmccullough wrote: > > Okay this appears to work when I run with -r (prints some crrev links) but I > get > > this error: > > > > Looking up activity..... > > auth.AuthenticationError: Access to https://chromereviews.googleplex.com is > > denied (server returned HTTP 403). > > > > depot-tools-auth login http://codereviews.googleplex.com doesn't seem to help. > > > Most probably API endpoint that is used by rietveld_search is not whitelisted > for OAuth access on http://chromereviews.googleplex.com. See (and modify if > appropriate) > https://chrome-internal.googlesource.com/infra/infra_internal/+/master/appeng... > > > https://codereview.chromium.org/1176243002/diff/1/auth.py > > File auth.py (right): > > > > https://codereview.chromium.org/1176243002/diff/1/auth.py#newcode223 > > auth.py:223: if hostname == 'https://code.google.com': > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > make it a top-level config-like dict, so it looks less like a hack :) Also > use > > > parsed.netloc (will be "code.google.com") as a key, since that's what used > in > > > other places: > > > > > > ADDITIONAL_SCOPES = { > > > 'code.google.com': 'https://www.googleapis.com/auth/projecthosting', > > > } > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py > > File my_activity.py (right): > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode106 > > my_activity.py:106: 'host': 'gerrit.chromium.org', > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > this host has been turn off today, can be removed from this list > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode139 > > my_activity.py:139: def get_auth_token(email): > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > this should be eventually deleted, it's broken > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode236 > > my_activity.py:236: cookie_file = > > os.path.expanduser('~/.codereview_upload_cookies') > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > it's not directly related to OAuth http://code.google.com issue, but this > code > > is > > > broken too. ~/.codereview_upload_cookies no longer exists. > > > > removed it, now it just checks "has_cookie" > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode251 > > my_activity.py:251: def has_cookie(instance): > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > this can be reformulated as: > > > > > > a = auth.get_authenticator_for_host(<url>) > > > return a.has_cached_credentials() > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode470 > > my_activity.py:470: "https://code.google.com", auth_config) > > On 2015/06/11 01:37:32, Vadim Sh. wrote: > > > just "code.google.com" here should work too > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode479 > > my_activity.py:479: 'q': '%s' % user_str, > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > 'q': user_str > > > > Done. > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode484 > > my_activity.py:484: _, body = http.request(url) > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > this thing will raise auth.AuthenticationError if there's no cached token. > > Catch > > > it somewhere in main() and ask user to login. Just printing the exception to > > > stdout should be enough. It asks to run depot-tools-auth > > > > > > (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/auth.py#94). > > > > Done. Will check and follow up with a separate CL (infra_internal is a different repo, correct?), if that's the way to fix it.
On 2015/06/11 02:26:10, seanmccullough wrote: > On 2015/06/11 02:20:58, Vadim Sh. wrote: > > lgtm > > > > On 2015/06/11 02:05:23, seanmccullough wrote: > > > Okay this appears to work when I run with -r (prints some crrev links) but I > > get > > > this error: > > > > > > Looking up activity..... > > > auth.AuthenticationError: Access to https://chromereviews.googleplex.com is > > > denied (server returned HTTP 403). > > > > > > depot-tools-auth login http://codereviews.googleplex.com doesn't seem to > help. > > > > > Most probably API endpoint that is used by rietveld_search is not whitelisted > > for OAuth access on http://chromereviews.googleplex.com. See (and modify if > > appropriate) > > > https://chrome-internal.googlesource.com/infra/infra_internal/+/master/appeng... > > > > > https://codereview.chromium.org/1176243002/diff/1/auth.py > > > File auth.py (right): > > > > > > https://codereview.chromium.org/1176243002/diff/1/auth.py#newcode223 > > > auth.py:223: if hostname == 'https://code.google.com': > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > make it a top-level config-like dict, so it looks less like a hack :) Also > > use > > > > parsed.netloc (will be "code.google.com") as a key, since that's what used > > in > > > > other places: > > > > > > > > ADDITIONAL_SCOPES = { > > > > 'code.google.com': 'https://www.googleapis.com/auth/projecthosting', > > > > } > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py > > > File my_activity.py (right): > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode106 > > > my_activity.py:106: 'host': 'gerrit.chromium.org', > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > this host has been turn off today, can be removed from this list > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode139 > > > my_activity.py:139: def get_auth_token(email): > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > this should be eventually deleted, it's broken > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode236 > > > my_activity.py:236: cookie_file = > > > os.path.expanduser('~/.codereview_upload_cookies') > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > it's not directly related to OAuth http://code.google.com issue, but this > > code > > > is > > > > broken too. ~/.codereview_upload_cookies no longer exists. > > > > > > removed it, now it just checks "has_cookie" > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode251 > > > my_activity.py:251: def has_cookie(instance): > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > this can be reformulated as: > > > > > > > > a = auth.get_authenticator_for_host(<url>) > > > > return a.has_cached_credentials() > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode470 > > > my_activity.py:470: "https://code.google.com", auth_config) > > > On 2015/06/11 01:37:32, Vadim Sh. wrote: > > > > just "code.google.com" here should work too > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode479 > > > my_activity.py:479: 'q': '%s' % user_str, > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > 'q': user_str > > > > > > Done. > > > > > > https://codereview.chromium.org/1176243002/diff/1/my_activity.py#newcode484 > > > my_activity.py:484: _, body = http.request(url) > > > On 2015/06/11 01:37:31, Vadim Sh. wrote: > > > > this thing will raise auth.AuthenticationError if there's no cached token. > > > Catch > > > > it somewhere in main() and ask user to login. Just printing the exception > to > > > > stdout should be enough. It asks to run depot-tools-auth > > > > > > > > > > (https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/auth.py#94). > > > > > > Done. > > Will check and follow up with a separate CL (infra_internal is a different repo, > correct?), if that's the way to fix it. yeah
The CQ bit was checked by seanmccullough@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1176243002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: depot_tools_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/depot_tools_pre...)
seanmccullough@chromium.org changed reviewers: + stip@chromium.org
+stip for OWNERS
OWNER lgtm
The CQ bit was checked by seanmccullough@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1176243002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=295626 |
