|
|
| LEFT | RIGHT |
|---|---|
| 1 # Copyright 2008 Google Inc. | 1 # Copyright 2008 Google Inc. |
| 2 # | 2 # |
| 3 # Licensed under the Apache License, Version 2.0 (the "License"); | 3 # Licensed under the Apache License, Version 2.0 (the "License"); |
| 4 # you may not use this file except in compliance with the License. | 4 # you may not use this file except in compliance with the License. |
| 5 # You may obtain a copy of the License at | 5 # You may obtain a copy of the License at |
| 6 # | 6 # |
| 7 # http://www.apache.org/licenses/LICENSE-2.0 | 7 # http://www.apache.org/licenses/LICENSE-2.0 |
| 8 # | 8 # |
| 9 # Unless required by applicable law or agreed to in writing, software | 9 # Unless required by applicable law or agreed to in writing, software |
| 10 # distributed under the License is distributed on an "AS IS" BASIS, | 10 # distributed under the License is distributed on an "AS IS" BASIS, |
| (...skipping 1728 matching lines...) | | Loading... | |
| 1739 | 1739 |
| 1740 ps_key = db.Key.from_path( | 1740 ps_key = db.Key.from_path( |
| 1741 models.PatchSet.kind(), | 1741 models.PatchSet.kind(), |
| 1742 db.allocate_ids(db.Key.from_path(models.PatchSet.kind(), 1, | 1742 db.allocate_ids(db.Key.from_path(models.PatchSet.kind(), 1, |
| 1743 parent=issue.key()), 1)[0], | 1743 parent=issue.key()), 1)[0], |
| 1744 parent=issue.key()) | 1744 parent=issue.key()) |
| 1745 | 1745 |
| 1746 patchset = models.PatchSet(issue=issue, data=data, url=url, key=ps_key) | 1746 patchset = models.PatchSet(issue=issue, data=data, url=url, key=ps_key) |
| 1747 patchset.put() | 1747 patchset.put() |
| 1748 | 1748 |
| 1749 commitPatch = _create_commit_message_patch(patchset, | 1749 commit_patch = _create_commit_message_patch(patchset, |
| 1750 patchset.issue.description) | 1750 patchset.issue.description) |
| 1751 commitPatch.put() | 1751 commit_patch.put() |
| 1752 | 1752 |
| 1753 if not separate_patches: | 1753 if not separate_patches: |
| 1754 try: | 1754 try: |
| 1755 patches = engine.ParsePatchSet(patchset) | 1755 patches = engine.ParsePatchSet(patchset) |
| 1756 except: | 1756 except: |
| 1757 # catch all exceptions happening in engine.ParsePatchSet, | 1757 # catch all exceptions happening in engine.ParsePatchSet, |
| 1758 # engine.SplitPatch. With malformed diffs a variety of exceptions could | 1758 # engine.SplitPatch. With malformed diffs a variety of exceptions could |
| 1759 # happen there. | 1759 # happen there. |
| 1760 logging.exception('Exception during patch parsing') | 1760 logging.exception('Exception during patch parsing') |
| 1761 patches = [] | 1761 patches = [] |
| (...skipping 65 matching lines...) | | Loading... | |
| 1827 @post_required | 1827 @post_required |
| 1828 @issue_editor_required | 1828 @issue_editor_required |
| 1829 @xsrf_required | 1829 @xsrf_required |
| 1830 def add(request): | 1830 def add(request): |
| 1831 """/<issue>/add - Add a new PatchSet to an existing Issue.""" | 1831 """/<issue>/add - Add a new PatchSet to an existing Issue.""" |
| 1832 issue = request.issue | 1832 issue = request.issue |
| 1833 form = AddForm(request.POST, request.FILES) | 1833 form = AddForm(request.POST, request.FILES) |
| 1834 if not _add_patchset_from_form(request, issue, form): | 1834 if not _add_patchset_from_form(request, issue, form): |
| 1835 return show(request, issue.key().id(), form) | 1835 return show(request, issue.key().id(), form) |
| 1836 return HttpResponseRedirect(reverse(show, args=[issue.key().id()])) | 1836 return HttpResponseRedirect(reverse(show, args=[issue.key().id()])) |
| 1837 | 1837 |
|
M-A
2013年08月20日 01:24:29
2 lines between file level symbols (and line 1854)
2 lines between file level symbols (and line 1854)
jparent1
2013年08月27日 22:46:39
Done.
On 2013年08月20日 01:24:29, M-A wrote:
> 2 lines between file level symbols (and line 1854)
Done.
| |
| 1838 | |
| 1838 def _create_commit_message_patch(patchset, description): | 1839 def _create_commit_message_patch(patchset, description): |
| 1839 diff = "" | 1840 diff = "" |
| 1840 udiff = difflib.unified_diff("",description.splitlines(1),· | 1841 udiff = difflib.unified_diff("",description.splitlines(1),· |
|
M-A
2013年08月20日 01:24:29
whitespace after comma.
whitespace after comma.
jparent1
2013年08月27日 22:46:39
Done.
On 2013年08月20日 01:24:29, M-A wrote:
> whitespace after comma.
Done.
| |
| 1841 fromfile="\dev\\null",tofile=COMMIT_MSG) | 1842 fromfile="/dev/null",tofile=COMMIT_MSG) |
|
iannucci
2013年08月19日 21:56:47
you can use a raw string to avoid the tricky backs
you can use a raw string to avoid the tricky backslashing: r'\dev\null'
Andi
2013年08月26日 09:39:42
Why do we use backslashes here at all? At least Me
On 2013年08月19日 21:56:47, iannucci (OOO through Aug 27) wrote:
> you can use a raw string to avoid the tricky backslashing: r'\dev\null'
Why do we use backslashes here at all? At least Mercurial uses "/dev/null"
jparent1
2013年08月27日 22:46:39
Switched to /dev/null
On 2013年08月26日 09:39:42, And
Switched to /dev/null
On 2013年08月26日 09:39:42, Andi wrote:
> On 2013年08月19日 21:56:47, iannucci (OOO through Aug 27) wrote:
> > you can use a raw string to avoid the tricky backslashing: r'\dev\null'
>
> Why do we use backslashes here at all? At least Mercurial uses "/dev/null"
| |
| 1842 for line in udiff: | 1843 for line in udiff: |
| 1843 diff += line | 1844 diff += line |
| 1844 | 1845 |
| 1845 patch_key = db.Key.from_path( | 1846 patch_key = db.Key.from_path( |
| 1846 models.Patch.kind(), | 1847 models.Patch.kind(), |
| 1847 db.allocate_ids(db.Key.from_path(models.Patch.kind(), 1, | 1848 db.allocate_ids(db.Key.from_path(models.Patch.kind(), 1, |
| 1848 parent=patchset.key()), 1)[0], | 1849 parent=patchset.key()), 1)[0], |
| 1849 parent=patchset.key()) | 1850 parent=patchset.key()) |
| 1850 commitPatch = models.Patch(patchset=patchset, text=utils.to_dbtext(diff),· | 1851 commit_patch = models.Patch(patchset=patchset, text=utils.to_dbtext(diff),· |
| 1851 filename=COMMIT_MSG, key=patch_key, | 1852 filename=COMMIT_MSG, key=patch_key, |
| 1852 no_base_file=True) | 1853 no_base_file=True) |
| 1853 return commitPatch | 1854 return commit_patch |
| 1855 | |
| 1854 | 1856 |
| 1855 def _add_patchset_from_form(request, issue, form, message_key='message', | 1857 def _add_patchset_from_form(request, issue, form, message_key='message', |
| 1856 emails_add_only=False): | 1858 emails_add_only=False): |
| 1857 """Helper for add() and upload().""" | 1859 """Helper for add() and upload().""" |
| 1858 if form.is_valid(): | 1860 if form.is_valid(): |
| 1859 data_url = _get_data_url(form) | 1861 data_url = _get_data_url(form) |
| 1860 if not form.is_valid(): | 1862 if not form.is_valid(): |
| 1861 return None | 1863 return None |
| 1862 account = models.Account.get_account_for_user(request.user) | 1864 account = models.Account.get_account_for_user(request.user) |
| 1863 if account.blocked: | 1865 if account.blocked: |
| 1864 return None | 1866 return None |
| 1865 if not issue.edit_allowed: | 1867 if not issue.edit_allowed: |
| 1866 # This check is done at each call site but check again as a safety measure. | 1868 # This check is done at each call site but check again as a safety measure. |
| 1867 return None | 1869 return None |
| 1868 data, url, separate_patches = data_url | 1870 data, url, separate_patches = data_url |
| 1869 message = form.cleaned_data[message_key] | 1871 message = form.cleaned_data[message_key] |
| 1870 ps_key = db.Key.from_path( | 1872 ps_key = db.Key.from_path( |
| 1871 models.PatchSet.kind(), | 1873 models.PatchSet.kind(), |
| 1872 db.allocate_ids(db.Key.from_path(models.PatchSet.kind(), 1, | 1874 db.allocate_ids(db.Key.from_path(models.PatchSet.kind(), 1, |
| 1873 parent=issue.key()), 1)[0], | 1875 parent=issue.key()), 1)[0], |
| 1874 parent=issue.key()) | 1876 parent=issue.key()) |
| 1875 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url, | 1877 patchset = models.PatchSet(issue=issue, message=message, data=data, url=url, |
| 1876 key=ps_key) | 1878 key=ps_key) |
| 1877 patchset.put() | 1879 patchset.put() |
| 1878 | 1880 |
| 1879 commitPatch = _create_commit_message_patch(patchset, | 1881 commit_patch = _create_commit_message_patch(patchset, |
|
M-A
2013年08月20日 01:24:29
golang style variable names? :) Usually use commit
golang style variable names? :) Usually use commit_patch style.
jparent1
2013年08月27日 22:46:39
Whoops. I'm a Google-style Javascript gal at heart
Whoops. I'm a Google-style Javascript gal at heart, so hard to break the habits
sometimes! Fixed here and elsewhere.
On 2013年08月20日 01:24:29, M-A wrote:
> golang style variable names? :) Usually use commit_patch style.
| |
| 1880 patchset.issue.description) | 1882 patchset.issue.description) |
| 1881 commitPatch.put() | 1883 commit_patch.put() |
| 1882 | 1884 |
| 1883 if not separate_patches: | 1885 if not separate_patches: |
| 1884 try: | 1886 try: |
| 1885 patches = engine.ParsePatchSet(patchset) | 1887 patches = engine.ParsePatchSet(patchset) |
| 1886 except: | 1888 except: |
| 1887 logging.exception('Exception during patchset parsing') | 1889 logging.exception('Exception during patchset parsing') |
| 1888 patches = [] | 1890 patches = [] |
| 1889 if not patches: | 1891 if not patches: |
| 1890 patchset.delete() | 1892 patchset.delete() |
| 1891 errkey = url and 'url' or 'data' | 1893 errkey = url and 'url' or 'data' |
| (...skipping 406 matching lines...) | | Loading... | |
| 2298 props[k] = value | 2300 props[k] = value |
| 2299 props.update(extra_args)···· | 2301 props.update(extra_args)···· |
| 2300 return klass(**props)···· | 2302 return klass(**props)···· |
| 2301 | 2303 |
| 2302 # Create a new patchset from the last one. | 2304 # Create a new patchset from the last one. |
| 2303 patchsets = list(issue.patchset_set.order('created')) | 2305 patchsets = list(issue.patchset_set.order('created')) |
| 2304 last_patchset = patchsets[-1] | 2306 last_patchset = patchsets[-1] |
| 2305 new_patchset_msg = 'Auto-generated patchset by description update.' | 2307 new_patchset_msg = 'Auto-generated patchset by description update.' |
| 2306 new_patchset = clone_without_key(last_patchset, parent=issue, | 2308 new_patchset = clone_without_key(last_patchset, parent=issue, |
| 2307 message=new_patchset_msg) | 2309 message=new_patchset_msg) |
| 2308 new_patchset.put() | 2310 new_patchset.put() |
|
iannucci
2013年08月19日 21:56:47
I'm a little worried about all these interleaved p
I'm a little worried about all these interleaved put()'s. appengine is pretty
notorious for spuriously emitting exceptions when interacting with the
datastore. The worrysome bit to me is if one of these put()'s throws, I think
the datastore will be in a weird state. However I'm not expert enough to be
confident in suggesting an answer (using a transaction springs to mind, but I'm
not sure if that will work).
M-A
2013年08月20日 01:24:29
Agreed, it's annoying when one of the put() throws
On 2013年08月19日 21:56:47, iannucci (OOO through Aug 27) wrote:
> I'm a little worried about all these interleaved put()'s. appengine is pretty
> notorious for spuriously emitting exceptions when interacting with the
> datastore. The worrysome bit to me is if one of these put()'s throws, I think
> the datastore will be in a weird state. However I'm not expert enough to be
> confident in suggesting an answer (using a transaction springs to mind, but
I'm
> not sure if that will work).
Agreed, it's annoying when one of the put() throws in the middle of a patchset.
It would make it slightly worse, worth a transaction (even if it'll make it even
more likely to fail)
jparent1
2013年08月27日 22:46:39
So, there is a bit of a problem, you can't allocat
So, there is a bit of a problem, you can't allocate ids inside of transactions
(see a recent change like https://codereview.appspot.com/10434044 to remove
transactions to deal with this). I need to allocate the id for the commit_patch
AFTER I create new_patchset, since it uses the parent patchset to create the
correct key.
I could wrap new_patchset creation + put in a try, and then do the rest of the
puts in a txn if putting new_patchset succeeds?
On 2013年08月20日 01:24:29, M-A wrote:
> On 2013年08月19日 21:56:47, iannucci (OOO through Aug 27) wrote:
> > I'm a little worried about all these interleaved put()'s. appengine is
pretty
> > notorious for spuriously emitting exceptions when interacting with the
> > datastore. The worrysome bit to me is if one of these put()'s throws, I
think
> > the datastore will be in a weird state. However I'm not expert enough to be
> > confident in suggesting an answer (using a transaction springs to mind, but
> I'm
> > not sure if that will work).
>
> Agreed, it's annoying when one of the put() throws in the middle of a
patchset.
> It would make it slightly worse, worth a transaction (even if it'll make it
even
> more likely to fail)
| |
| 2309 · | 2311 · |
| 2310 # Add the new commit message patch. | 2312 # Add the new commit message patch. |
| 2311 commitPatch = _create_commit_message_patch(new_patchset, issue.description) | 2313 commit_patch = _create_commit_message_patch(new_patchset, issue.description) |
| 2312 commitPatch.put() | 2314 commit_patch.put() |
| 2313 | 2315 |
| 2314 # And copy all the patches over from last patchset. | 2316 # And copy all the patches over from last patchset. |
| 2315 for patch in list(last_patchset.patch_set): | 2317 for patch in list(last_patchset.patch_set): |
| 2316 # Skip the old commit message, since we just created a new one. | 2318 # Skip the old commit message, since we just created a new one. |
| 2317 if patch.filename == COMMIT_MSG: | 2319 if patch.filename == COMMIT_MSG: |
| 2318 continue | 2320 continue |
| 2319 new_patch = clone_without_key(patch, parent=new_patchset, | 2321 new_patch = clone_without_key(patch, parent=new_patchset, |
| 2320 patchset=new_patchset) | 2322 patchset=new_patchset) |
| 2321 new_patch.put() | 2323 new_patch.put() |
| 2322 | 2324 |
| (...skipping 2246 matching lines...) | | Loading... | |
| 4569 if form.is_valid(): | 4571 if form.is_valid(): |
| 4570 client_id = form.cleaned_data['client_id'] | 4572 client_id = form.cleaned_data['client_id'] |
| 4571 client_secret = form.cleaned_data['client_secret'] | 4573 client_secret = form.cleaned_data['client_secret'] |
| 4572 additional_client_ids = form.cleaned_data['additional_client_ids'] | 4574 additional_client_ids = form.cleaned_data['additional_client_ids'] |
| 4573 auth_utils.SecretKey.set_config(client_id, client_secret, | 4575 auth_utils.SecretKey.set_config(client_id, client_secret, |
| 4574 additional_client_ids) | 4576 additional_client_ids) |
| 4575 return HttpResponseRedirect(reverse(set_client_id_and_secret)) | 4577 return HttpResponseRedirect(reverse(set_client_id_and_secret)) |
| 4576 else: | 4578 else: |
| 4577 form = ClientIDAndSecretForm() | 4579 form = ClientIDAndSecretForm() |
| 4578 return respond(request, 'set_client_id_and_secret.html', {'form': form}) | 4580 return respond(request, 'set_client_id_and_secret.html', {'form': form}) |
| LEFT | RIGHT |