Issue 4095042: Fix upload.py link when Django app is running on production (fixes issue 25) (Closed)
|
Created:
15 years, 4 months ago by techtonik Modified:
15 years, 4 months ago Reviewers:
Andi Albrecht CC:
gae2django_googlegroups.com Base URL:
http://django-gae2django.googlecode.com/svn/trunk/ Visibility:
Public. |
Patch Set 1 #
Total comments: 5
Patch Set 2 : Update Makefile to fetch latest changes #
Total comments: 1
MessagesTotal messages: 14
http://codereview.appspot.com/4095042/diff/1/examples/rietveld/README File examples/rietveld/README (right): http://codereview.appspot.com/4095042/diff/1/examples/rietveld/README#newcode69 examples/rietveld/README:69: patch -p0 < download.link.diff This should go into the Makefile too (sorry, the last one too, I've missed that). http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... File examples/rietveld/patches/download.link.diff (right): http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... examples/rietveld/patches/download.link.diff:10: + (r'^dynamic/upload.py$', 'customized_upload_py'), /static/upload.py is still linked from the HTML pages, e.g. when clicking on "Create Issue".
Sign in to reply to this message.
http://codereview.appspot.com/4095042/diff/1/examples/rietveld/README File examples/rietveld/README (right): http://codereview.appspot.com/4095042/diff/1/examples/rietveld/README#newcode69 examples/rietveld/README:69: patch -p0 < download.link.diff On 2011/01/22 07:27:33, Andi Albrecht wrote: > This should go into the Makefile too (sorry, the last one too, I've missed > that). Done. http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... File examples/rietveld/patches/download.link.diff (right): http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... examples/rietveld/patches/download.link.diff:10: + (r'^dynamic/upload.py$', 'customized_upload_py'), On 2011/01/22 07:27:33, Andi Albrecht wrote: > /static/upload.py is still linked from the HTML pages, e.g. when clicking on > "Create Issue". I've updated Makefile with a newer revision of Rietveld. This should fix the issue.
Sign in to reply to this message.
http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... File examples/rietveld/patches/download.link.diff (right): http://codereview.appspot.com/4095042/diff/1/examples/rietveld/patches/downlo... examples/rietveld/patches/download.link.diff:10: + (r'^dynamic/upload.py$', 'customized_upload_py'), On 2011/01/22 09:05:16, techtonik wrote: > On 2011/01/22 07:27:33, Andi Albrecht wrote: > > /static/upload.py is still linked from the HTML pages, e.g. when clicking on > > "Create Issue". > > I've updated Makefile with a newer revision of Rietveld. This should fix the > issue. hm, I don't see how updating to a newer version fixes this. In newer versions of Rietveld "/static/upload.py" is replaced by "{{ media_url }}upload.py" which is contradictory to your change. As far as I understand your change you don't want upload.py to be served from {{media_url}}, isn't it? http://codereview.appspot.com/4095042/diff/6001/examples/rietveld/Makefile File examples/rietveld/Makefile (right): http://codereview.appspot.com/4095042/diff/6001/examples/rietveld/Makefile#ne... examples/rietveld/Makefile:1: RIETVELDREV=650 Updating the revision isn't that easy. The drawback is that the patches need to be updated too. When I apply this patch and run "make all" two patches fail. Actually that's the reason why I don't use always the trunk version - read "RIETVELDREV=xxx" as "known to be working with rev xxx" :)
Sign in to reply to this message.
On 2011/01/22 09:12:59, Andi Albrecht wrote:
> In newer versions of Rietveld "/static/upload.py" is replaced by "{{ media_url
> }}upload.py" which is contradictory to your change. As far as I understand
your
> change you don't want upload.py to be served from {{media_url}}, isn't it?
Yes, but r644 changed {{media_url}} to {%url
codereview.views.customized_upload_py%}"> -
http://code.google.com/p/rietveld/source/browse/trunk/templates/new.html#11 ;)
> examples/rietveld/Makefile:1: RIETVELDREV=650
> Updating the revision isn't that easy. The drawback is that the patches need
to
> be updated too. When I apply this patch and run "make all" two patches fail.
> Actually that's the reason why I don't use always the trunk version - read
> "RIETVELDREV=xxx" as "known to be working with rev xxx" :)
That's a pain. May these patches fail, because already patched? Or because
clear_rietveld should be run first?
Sign in to reply to this message.
On Sat, Jan 22, 2011 at 10:35 AM, <techtonik@gmail.com> wrote: > On 2011/01/22 09:12:59, Andi Albrecht wrote: >> >> In newer versions of Rietveld "/static/upload.py" is replaced by "{{ > > media_url >> >> }}upload.py" which is contradictory to your change. As far as I > > understand your >> >> change you don't want upload.py to be served from {{media_url}}, isn't > > it? > > Yes, but r644 changed {{media_url}} to {%url > codereview.views.customized_upload_py%}"> - > http://code.google.com/p/rietveld/source/browse/trunk/templates/new.html#11 > ;) sorry, my mistake. My working copy wasn't up to date on this machine. You're right, of course :) > >> examples/rietveld/Makefile:1: RIETVELDREV=650 >> Updating the revision isn't that easy. The drawback is that the > > patches need to >> >> be updated too. When I apply this patch and run "make all" two patches > > fail. >> >> Actually that's the reason why I don't use always the trunk version - > > read >> >> "RIETVELDREV=xxx" as "known to be working with rev xxx" :) > > That's a pain. May these patches fail, because already patched? Or > because clear_rietveld should be run first? The patches fail because the don't apply properly anymore. And yes, it's best to run clear_rietveld to force the whole procedure to start over again. > > > http://codereview.appspot.com/4095042/ >
Sign in to reply to this message.
On 2011/01/22 09:40:46, Andi Albrecht wrote: > > > >> examples/rietveld/Makefile:1: RIETVELDREV=650 > >> Updating the revision isn't that easy. The drawback is that the > > > > patches need to > >> > >> be updated too. When I apply this patch and run "make all" two patches > > > > fail. > >> > >> Actually that's the reason why I don't use always the trunk version - > > > > read > >> > >> "RIETVELDREV=xxx" as "known to be working with rev xxx" :) > > > > That's a pain. May these patches fail, because already patched? Or > > because clear_rietveld should be run first? > > The patches fail because the don't apply properly anymore. And yes, > it's best to run clear_rietveld to force the whole procedure to start > over again. That's strange. I've just applied them manually to Rietveld trunk without any conflicts, but I work on Windows. Have you tried 'svn st codereview/ templates/' to see if there are any conflicts unresolved already in these files? > > > > http://codereview.appspot.com/4095042/ > >
Sign in to reply to this message.
On Sat, Jan 22, 2011 at 11:16 AM, <techtonik@gmail.com> wrote: > On 2011/01/22 09:40:46, Andi Albrecht wrote: >> >> > >> >> examples/rietveld/Makefile:1: RIETVELDREV=650 >> >> Updating the revision isn't that easy. The drawback is that the >> > >> > patches need to >> >> >> >> be updated too. When I apply this patch and run "make all" two > > patches >> >> > >> > fail. >> >> >> >> Actually that's the reason why I don't use always the trunk version > > - >> >> > >> > read >> >> >> >> "RIETVELDREV=xxx" as "known to be working with rev xxx" :) >> > >> > That's a pain. May these patches fail, because already patched? Or >> > because clear_rietveld should be run first? > >> The patches fail because the don't apply properly anymore. And yes, >> it's best to run clear_rietveld to force the whole procedure to start >> over again. > > That's strange. I've just applied them manually to Rietveld trunk > without any conflicts, but I work on Windows. Have you tried 'svn st > codereview/ templates/' to see if there are any conflicts unresolved > already in these files? This is what happens here (current SVN with your patch applied): $ make all svn export http://rietveld.googlecode.com/svn/trunk/upload.py@650 A upload.py Export complete. patch -p0 < patches/upload.diff patching file upload.py patch -p0 < patches/account-login-links.diff patching file templates/issue_base.html Hunk #1 FAILED at 28. 1 out of 1 hunk FAILED -- saving rejects to file templates/issue_base.html.rej patching file templates/base.html Hunk #1 FAILED at 168. 1 out of 1 hunk FAILED -- saving rejects to file templates/base.html.rej make: *** [upload.py] Error 1 > >> > >> > http://codereview.appspot.com/4095042/ >> > > > > > http://codereview.appspot.com/4095042/ >
Sign in to reply to this message.
On 2011/01/22 10:22:40, Andi Albrecht wrote: > This is what happens here (current SVN with your patch applied): > > $ make all > svn export http://rietveld.googlecode.com/svn/trunk/upload.py%40650 > A upload.py > Export complete. > patch -p0 < patches/upload.diff > patching file upload.py > patch -p0 < patches/account-login-links.diff > patching file templates/issue_base.html > Hunk #1 FAILED at 28. > 1 out of 1 hunk FAILED -- saving rejects to file templates/issue_base.html.rej > patching file templates/base.html > Hunk #1 FAILED at 168. > 1 out of 1 hunk FAILED -- saving rejects to file templates/base.html.rej > make: *** [upload.py] Error 1 I don't have any other advice than to run `make clean_rietveld all` and check that files `svn st templates/` is clean. There is no other logical explanation. Manual checkout and patch works ok. Just tested this again.
Sign in to reply to this message.
On Sat, Jan 22, 2011 at 11:43 AM, <techtonik@gmail.com> wrote: > On 2011/01/22 10:22:40, Andi Albrecht wrote: >> >> This is what happens here (current SVN with your patch applied): > >> $ make all >> svn export http://rietveld.googlecode.com/svn/trunk/upload.py%40650 >> A upload.py >> Export complete. >> patch -p0 < patches/upload.diff >> patching file upload.py >> patch -p0 < patches/account-login-links.diff >> patching file templates/issue_base.html >> Hunk #1 FAILED at 28. >> 1 out of 1 hunk FAILED -- saving rejects to file > > templates/issue_base.html.rej >> >> patching file templates/base.html >> Hunk #1 FAILED at 168. >> 1 out of 1 hunk FAILED -- saving rejects to file > > templates/base.html.rej >> >> make: *** [upload.py] Error 1 > > I don't have any other advice than to run `make clean_rietveld all` and > check that files `svn st templates/` is clean. There is no other logical > explanation. Manual checkout and patch works ok. Just tested this again. ok, I've got it. When running make all the patches are applied (during "upload.py" target) *before* templates/ is checked out. Do you have make installed so you can fix the build order? If not, just commit it and I'll fix it later. Andi > > > http://codereview.appspot.com/4095042/ >
Sign in to reply to this message.
I've committed the patch to r163. Unfortunately, there is no `make` on my Vista.
Sign in to reply to this message.
ok, "make all" works again (r164). On Sat, Jan 22, 2011 at 10:58 PM, <techtonik@gmail.com> wrote: > I've committed the patch to r163. Unfortunately, there is no `make` on > my Vista. > > http://codereview.appspot.com/4095042/ >
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
