Merge lp:~widelands-dev/widelands-website/add_DISPLAY_to_update_help into lp:widelands-website

[kaputtnik]

Use DISPLAY=:1 for update_help.py because wl_map_object_info needs access to the x-server on DISPLAY:1. So

./manage.py update_help

runs without setting the environment variable before by hand.

[SirVer (sirver)]

I suggest crashing if DISPLAY is unset and instead make sure it is set correctly in the cron script that will trigger this run. The proposed change is fine as well, but if we ever need to shell out to another graphical process again, we have to repeat the hack. The cron script is where all commands are triggered, so it seems a better place for it, IMHO.

Right now all django commands are run through /etc/cron.daily/django_regular_commands.

[kaputtnik (franku)]

I am not sure if unsetting DISPLAY is needed at all.

The cron script runs only the commands for the productive website, as i can see. I try it this evening to update the productive website and set the environment variable in the cron script.

[kaputtnik (franku)]

After i wrote you the PN a few days ago i have reverted the changes to /etc/cron.daily/django_regular_commands. I couldn't get it to work.

I have managed now to update the help by hand, except the update_help_pdf. The latter one complains about "Operation not permitted:" when writing the pdf (i don't know why, the file permissions seems to be ok). Trying to run the script as user www-data fails also.

So the current state is:
1. The Encyclopedia on the website is fine now
2. The economy graph pdf files are not updated/correct
3. There is currently no method available to get the help updated through a cron-job

What to do?

The proposed branch where tested on the alpha site and i think these changes are currently the only ones which should work for updating the help through a cron-job. Regarding unsetting DISPLAY i think it is not really needed, because setting/unsetting it through os.environ works not globally... but i haven't test it right now though.

The other possibility is to make a button/link on the encyclopedia main page to update the help. But running a cronjob is definitely better.

[GunChleoc (gunchleoc)]

There must be a configuration error with the paths somewhere - the "w" in "wlhelp" is missing for the image path links on the live website. I did a grep over trunk and didn't find any errors there, so it must be a configuration issue.

Expected link:
https://wl.widelands.org/wlmedia/wlhelp/img/atlanteans/atlanteans_headquarters/menu.png

Actual link:
https://wl.widelands.org/wlmedia/lhelp/img/atlanteans/atlanteans_headquarters/menu.png

Thanks for doing all this work - I'm a bit MIA right now, because the Gaelic Wikipedia urgently needs my help with some user interface stuff.

[kaputtnik (franku)]

Uhh i didn't checked if the output is really good :-( Thanks for looking into this.

I think i have found it... The MEDIA_ROOT path contained double slashes. I don't know why, but this causes the slice in line 154 of update_help.py to cut too many chars:

return '%s%s' % (MEDIA_URL, new_name[len(MEDIA_ROOT):])

This is fixed now for all STATIC strings in local_settings.py and the encyclopedia on the website is updated again.

You may notice that i removed a slash in between the "%s". I did that because MEDIA_URL contains a slash itself and putting in there a slash results in a doubleslash again.

[GunChleoc (gunchleoc)]

Thanks for cleaning up after me!

Running os.path.normpath(directory) should take care of any double slashes - this is how I fixed up things during the last code review round. Looks like we have missed a few :(

Are you still having problems with the pdf? Maybe it's also a path issue.

[kaputtnik (franku)]

> Running os.path.normpath(directory) should take care of any double slashes -
> this is how I fixed up things during the last code review round. Looks like we
> have missed a few :(
 I think having paths with double slashes should be fixed where they are build. Failures like this one would otherwise be triggered again and again when one writes new code.

> Are you still having problems with the pdf?
I have managed to fixed this too by renaming "media/wlhelp/network_graphs" and run

./manage.py update_help_pdf

again. This time it was running through without problems.

So all should be fine now on the productive website. What remains is then to manage the automatic update of the help...

[GunChleoc (gunchleoc)]

> > Running os.path.normpath(directory) should take care of any double slashes -
> > this is how I fixed up things during the last code review round. Looks like
> we
> > have missed a few :(
> I think having paths with double slashes should be fixed where they are
> build. Failures like this one would otherwise be triggered again and again
> when one writes new code.

Agreed - ron the normpath ove it at the place that it is built if possible

> > Are you still having problems with the pdf?
> I have managed to fixed this too by renaming "media/wlhelp/network_graphs" and
> run
>
> ./manage.py update_help_pdf
>
> again. This time it was running through without problems.

Looking good :)

> So all should be fine now on the productive website. What remains is then to
> manage the automatic update of the help...

I don't think we really need an automatic update here - this data doesn't change very often. Maybe as a wishlist bug?

[kaputtnik (franku)]

> Agreed - ron the normpath ove it at the place that it is built if possible

Hm, i don't know... running normpath over each path constant after it has been build?:

bd = "/path/to/some/thing/"
MEDIA = bd + "/extended/path/"
MEDIA = os.path.normpath(MEDIA)

Looks like the programmer doesn't trust himself :-D

> I don't think we really need an automatic update here - this data doesn't
> change very often. Maybe as a wishlist bug?

I have added a task for updating the encyclopedia to https://wl.widelands.org/wiki/ReleasingWidelands/

Nevertheless we should have a hint for the DISPLAY problem. The easiest way would be to modify the error message from:

"Error: Unable to execute 'wl_map_object_info' for generating the JSON files."

to:

"Error: Unable to execute 'wl_map_object_info' for generating the JSON files. Have you exported DISPLAY = ':1' before running update_help?"

[GunChleoc (gunchleoc)]

> "Error: Unable to execute 'wl_map_object_info' for generating the JSON files. Have you exported DISPLAY = ':1' before running update_help?"

+1

Unmerged revisions

407. By kaputtnik on 2016-05-25

add environment variable DISPLAY = :1 for update_help.py

406. By kaputtnik on 2016-05-23

merged with trunk

=== modified file 'wlhelp/management/commands/update_help.py'
--- wlhelp/management/commands/update_help.py	2016-05-22 11:35:57 +0000
+++ wlhelp/management/commands/update_help.py	2016-05-25 18:51:24 +0000
@@ -291,6 +291,14 @@
         current_dir = os.path.dirname(os.path.realpath(__file__))
         is_json_valid = False
         os.chdir(WIDELANDS_SVN_DIR)
+        # Because wl_map_object_info needs access to a graphical server, which is running at
+        # DISPLAY :1 on the productive server, we set the environment variable
+        # accordingly.
+        display_set = False
+        if not 'DISPLAY' in os.environ:
+            # No DISPLAY set -> default on the server console
+            display_set = True
+            os.environ['DISPLAY'] = ':1'
         try:
             subprocess.check_call(
                 [os.path.normpath('wl_map_object_info'), json_directory])
@@ -298,6 +306,10 @@
             print(
                 "Error: Unable to execute 'wl_map_object_info' for generating the JSON files.")
 
+        if display_set:
+            # Unset DISPLAY
+            os.environ.pop('DISPLAY')
+
         # Now we validate that they are indeed JSON files (syntax check only)
         validator_script = os.path.normpath(
             WIDELANDS_SVN_DIR + '/utils/validate_json.py')