Merge lp:~milo/linaro-patchmetrics/bug1013577 into lp:linaro-patchmetrics

Proposed by Milo Casagrande
Status: Merged
Merged at revision: 367
Proposed branch: lp:~milo/linaro-patchmetrics/bug1013577
Merge into: lp:linaro-patchmetrics
Diff against target: 42 lines (+9/-4)
2 files modified
apps/patchmetrics/gerrit.py (+7/-2)
apps/patchmetrics/tests/test_gerrit.py (+2/-2)
To merge this branch: bzr merge lp:~milo/linaro-patchmetrics/bug1013577
Reviewer Review Type Date Requested Status
Deepti B. Kalakeri (community) Approve
Review via email: mp+112722@code.launchpad.net

Description of the change

Proposing another merge, hopefully now taking into consideration all cases.

We still have a problem with some type of "emails", that are not valid, like "dp583@stebjsxu0119.(none)": the android-review gerrit instance will skip the "(none)" part giving back an error string that does not contain the original email.

The changes should fix this, and also the tests.

To post a comment you must log in.
Revision history for this message
Deepti B. Kalakeri (deeptik) wrote :

+1 Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'apps/patchmetrics/gerrit.py'
--- apps/patchmetrics/gerrit.py 2012-06-27 10:35:27 +0000
+++ apps/patchmetrics/gerrit.py 2012-06-29 08:26:22 +0000
@@ -120,9 +120,14 @@
120 for key, value in query_params.iteritems():120 for key, value in query_params.iteritems():
121 # We cannot have None values here, we just skip them.121 # We cannot have None values here, we just skip them.
122 if value is not None:122 if value is not None:
123 # We need string, and remove white chars.
124 value = str(value).strip()
125 # Some emails need to be quoted, and only double quotes are
126 # accepted by gerrit.
127 if key == GERRIT_OWNER_KEY:
128 value = ("\"%s\"" % value)
123 # We need strings here, and we remove white spaces.129 # We need strings here, and we remove white spaces.
124 params_list.append(":".join((key,130 params_list.append(":".join((key, urllib.quote(value))))
125 urllib.quote(str(value).strip()))))
126 # The resulting query string, without separators or prefixes, should be in131 # The resulting query string, without separators or prefixes, should be in
127 # the form of:132 # the form of:
128 # key:value+key:value133 # key:value+key:value
129134
=== modified file 'apps/patchmetrics/tests/test_gerrit.py'
--- apps/patchmetrics/tests/test_gerrit.py 2012-06-27 12:17:02 +0000
+++ apps/patchmetrics/tests/test_gerrit.py 2012-06-29 08:26:22 +0000
@@ -112,7 +112,7 @@
112 query_params = {'owner': 'person@example.org', 'sortkey_after': 'abcd'}112 query_params = {'owner': 'person@example.org', 'sortkey_after': 'abcd'}
113 expected_result = (ROOT_REST_URL + CHANGES_QUERY_PATH +113 expected_result = (ROOT_REST_URL + CHANGES_QUERY_PATH +
114 QUERY_STRING_START + "n=100&format=JSON&q=owner:"114 QUERY_STRING_START + "n=100&format=JSON&q=owner:"
115 "person%40example.org+resume_sortkey:abcd")115 "%22person%40example.org%22+resume_sortkey:abcd")
116 self.assertEquals(expected_result,116 self.assertEquals(expected_result,
117 create_request_query_url(ROOT_REST_URL,117 create_request_query_url(ROOT_REST_URL,
118 CHANGES_QUERY_PATH,118 CHANGES_QUERY_PATH,
@@ -125,7 +125,7 @@
125 GERRIT_QUERY_RESUME_SORTKEY: 'abcd'}125 GERRIT_QUERY_RESUME_SORTKEY: 'abcd'}
126 expected_result = (ROOT_REST_URL + CHANGES_QUERY_PATH +126 expected_result = (ROOT_REST_URL + CHANGES_QUERY_PATH +
127 QUERY_STRING_START + "n=100&format=JSON&q=owner:"127 QUERY_STRING_START + "n=100&format=JSON&q=owner:"
128 "name%2Bsurname.person%40example.org+"128 "%22name%2Bsurname.person%40example.org%22+"
129 "resume_sortkey:abcd")129 "resume_sortkey:abcd")
130 self.assertEquals(expected_result,130 self.assertEquals(expected_result,
131 create_request_query_url(ROOT_REST_URL,131 create_request_query_url(ROOT_REST_URL,

Subscribers

People subscribed via source and target branches