Windows compatibility fixes

[GoogleCodeExporter]

The attached patch contains a couple of fixes needed to use djangotask in 
Windows. Namely,

 - The PYTHONPATH separator is ';' on windows, thus it must be set differently depending on the os.
 - Similarly, windows version of subprocess.Popen() doesn't support close_fds=True together with stdout/stderr redirections.
 - There are some hard coded Unix specific file names in the source, that must be dealt with. I couldn't really find good standard windows alternatives for them, thus:
   - I made django-taskd.pid file configurable with settings.TASKS_PID_FILE. This can actually be useful on linux, too.
   - The standard paths in become_daemon() were overwritten when used in taskd.py. It didn't really make sense even on Linux, thus I removed them.
 - Finally, I also fixed a buggy exception text in models.py. (It does not affect windows compatibility.)

Original issue reported on code.google.com by alpar.ju...@gmail.com on 18 Aug 2011 at 6:30

Attachments:

• win-fix.patch

[GoogleCodeExporter]

The patch looks good, with the following caveats:

 * I'm not 100% happy with
+            from django.conf import settings
+            daemon = TaskDaemon(settings.TASKS_PID_FILE
+                                if hasattr(settings, 'TASKS_PID_FILE')
+                                else '/tmp/django-taskd.pid')
there is no need for a setting, I think: it should be a fixed place on Windows. 
If I'm correct, the TEMP env var is always defined on Windows, so it should be 

join(os.getenv('TEMP'), 'django-taskd.pid')

with a good exception behind thrown if the getenv doesn't work. Yes, I googled 
a little, this is safe. So on Unix it should be always /tmp/django-taskd.pid, 
but on Windows it should be join(os.getenv('TEMP'), 'django-taskd.pid')

Then - instead of
+                env['PYTHONPATH'] = \
+                    (';' if os.name == 'nt' else ':').join(sys.path)
I would propose:
+                env['PYTHONPATH'] = os.pathsep.join(sys.path)

Finally, for
+                                        close_fds=os.name != 'nt',
I would rather write it
+                                        close_fds= (os.name != 'nt'),
for clarity

I would suggest we commit it, and we ping Alpar to test it ? since it wasn't 
working (and wasn't supported :) ) before, it's always better, even if it 
doesn't fix it yet

and Alpar.ju... can be the "official tester" on Windows :)

Original comment by farial...@gmail.com on 13 Sep 2011 at 9:44

[GoogleCodeExporter]

ok, patch applied in r49. Alpar, could you verify that it works for you ?

Original comment by farial...@gmail.com on 13 Sep 2011 at 10:05

• Changed state: Fixed

[GoogleCodeExporter]

> there is no need for a setting, I think: it should be a fixed place on 
Windows. If > I'm correct, the TEMP env var is always defined on Windows, so it 
should be 

Using the TEMP env is a good idea. In fact, it makes me think why don't we use 
$TMPDIR on LINUX?

On the other hand, I still think that a configurable pid file may be useful, 
even on Linux. For example you may want to run two djangotasks instances with 
two independent django site. Currently it is impossible because both would use 
the same pid file.

> I would propose:
> +                env['PYTHONPATH'] = os.pathsep.join(sys.path)

This is much better indeed.

> Finally, for
> +                                        close_fds=os.name != 'nt',
> I would rather write it
> +                                        close_fds= (os.name != 'nt'),
> for clarity

Fine for me, though simply I copied my version from django 1.3 (file 
management/commands/makemessages.py, line 45)

> ok, patch applied in r49. Alpar, could you verify that it works for you ?

Yes, it works for me.

> and Alpar.ju... can be the "official tester" on Windows :)

No way! I basically never use Windows and know nothing about it. The latest 
(and only) version that was really installed on my own computer was Windows 
3.1, way back in the early 90s.

Original comment by alpar.ju...@gmail.com on 14 Sep 2011 at 7:13