Merge lp:~rafalcieslak256/ubuntu-accomplishments-daemon/awesome-scriptrunner into lp:ubuntu-accomplishments-daemon
- awesome-scriptrunner
- Merge into accomplishments-daemon
Proposed by
Rafał Cieślak
Status: | Merged |
---|---|
Merged at revision: | 92 |
Proposed branch: | lp:~rafalcieslak256/ubuntu-accomplishments-daemon/awesome-scriptrunner |
Merge into: | lp:ubuntu-accomplishments-daemon |
Diff against target: |
425 lines (+114/-113) 4 files modified
accomplishments/daemon/api.py (+101/-105) accomplishments/daemon/dbusapi.py (+8/-3) accomplishments/daemon/service.py (+1/-1) po/accomplishments-daemon.pot (+4/-4) |
To merge this branch: | bzr merge lp:~rafalcieslak256/ubuntu-accomplishments-daemon/awesome-scriptrunner |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jono Bacon | Pending | ||
Review via email: mp+114016@code.launchpad.net |
Commit message
Description of the change
This introduces the new scriptrunner that uses producer-consumer model. It is compatible with current viewer, so there is no need to change anything there. New scriptrunner is much more efficient - it's an integral part of NewAPI, check it's specs for more details on scriptrunner.
Also, there is a new API call: run_script(accomID) that schedules a single script to be run. run_scripts(list) now takes a list of accomIDs as it's argument, if there is none, it will schedule everything appropriate (not completed & unlocked).
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'accomplishments/daemon/api.py' |
2 | --- accomplishments/daemon/api.py 2012-07-06 04:18:45 +0000 |
3 | +++ accomplishments/daemon/api.py 2012-07-09 17:35:12 +0000 |
4 | @@ -27,7 +27,8 @@ |
5 | import subprocess |
6 | import time |
7 | import locale |
8 | - |
9 | +from collections import deque |
10 | + |
11 | import dbus |
12 | import dbus.service |
13 | |
14 | @@ -70,7 +71,6 @@ |
15 | #flags used for scripts_state |
16 | NOT_RUNNING = 0 |
17 | RUNNING = 1 |
18 | -NEEDS_RE_RUNNING = 2 |
19 | |
20 | # XXX the source code needs to be updated to use Twisted async calls better: |
21 | # grep the source code for any *.asyncapi.* references, and if they return |
22 | @@ -81,11 +81,25 @@ |
23 | for better readability and separation of concerns. |
24 | """ |
25 | def __init__(self, parent): |
26 | + |
27 | self.parent = parent |
28 | + |
29 | + # The following variable represents state of scripts.Its state |
30 | + # can be either RUNNING or NOT_RUNNING. |
31 | + # The use of this flag is to aviod running several instances of |
32 | + # start_scriptrunner, which might result in undefined, troubleful |
33 | + # behavior. The flags are NOT_RUNNING by default, and are set to |
34 | + # RUNNING when the start_scriptrunner runs, and back to |
35 | + # NOT_RUNNING when it exits. This way, if the function is already |
36 | + # processing scripts, another calls will abort, having checked that |
37 | + # this flag is set to RUNNING. |
38 | + self.scripts_state = NOT_RUNNING |
39 | |
40 | @staticmethod |
41 | def run_a_subprocess(command): |
42 | - log.msg("Running subprocess command: " + str(command)) |
43 | + # Commented out this debug message, as it creates lots of junk, |
44 | + # and is not needed for common troubleshooting |
45 | + # log.msg("Running subprocess command: " + str(command)) |
46 | pprotocol = SubprocessReturnCodeProtocol() |
47 | reactor.spawnProcess(pprotocol, command[0], command, env=os.environ) |
48 | return pprotocol.returnCodeDeferred |
49 | @@ -173,35 +187,16 @@ |
50 | |
51 | # XXX let's rewrite this to use deferreds explicitly |
52 | @defer.inlineCallbacks |
53 | - def run_scripts_for_user(self, uid): |
54 | - # The following avoids running multiple instances of this function, |
55 | - # which might get very messy and cause a lot of trouble. Simulatnously |
56 | - # run scripts would be the case if user recieves several .asc files |
57 | - # within a short time, the scripts take extraordinary time to run, |
58 | - # or for various other reasons. |
59 | - # NOTE: detailed explanation of scripts_state mechanism is included |
60 | - # near it's initialisation in Accomplishments.__init__(...). |
61 | - if uid in self.parent.scripts_state: |
62 | - if self.parent.scripts_state[uid] is RUNNING: |
63 | - log.msg("Aborting running scripts, execution already in progress. Will re-do this when current run ends.") |
64 | - # scripts are already being run for that user, but since something |
65 | - # called that function, maybe we need to re-run them because |
66 | - # something has changed since last call, so let's schedule the |
67 | - # re-running immidiatelly after finishing this run, and abort |
68 | - self.parent.scripts_state[uid] = NEEDS_RE_RUNNING |
69 | - return |
70 | - elif self.parent.scripts_state[uid] is NEEDS_RE_RUNNING: |
71 | - log.msg("Aborting running scripts, execution already in progress. Re-runing scripts has already been scheduled.") |
72 | - # already scheduled, so just aborting |
73 | - return |
74 | - # if above conditions failed, that means the scripts are not being run |
75 | - # this user, so we can continue normally, marking the scripts as running... |
76 | - self.parent.scripts_state[uid] = RUNNING |
77 | - |
78 | - log.msg("--- Starting Running Scripts ---") |
79 | - timestart = time.time() |
80 | - self.parent.service.scriptrunner_start() |
81 | - |
82 | + def start_scriptrunner(self): |
83 | + |
84 | + # More info on scripts_state can be found in __init__ |
85 | + if self.scripts_state is RUNNING: |
86 | + # Aborting this call - scriptrunner is already working. |
87 | + return |
88 | + |
89 | + self.scripts_state = RUNNING |
90 | + |
91 | + uid = os.getuid() |
92 | # Is the user currently logged in and running a gnome session? |
93 | # XXX use deferToThread |
94 | username = pwd.getpwuid(uid).pw_name |
95 | @@ -214,7 +209,7 @@ |
96 | # user does not have gnome-session running or isn't logged in at |
97 | # all |
98 | log.msg("No gnome-session process for user %s" % username) |
99 | - self.parent.scripts_state[uid] = NOT_RUNNING #unmarking to avoid dead-lock |
100 | + self.scripts_state = NOT_RUNNING #unmarking to avoid dead-lock |
101 | return |
102 | # XXX this is a blocking call and can't be here if we want to take |
103 | # advantage of deferreds; instead, rewrite this so that the blocking |
104 | @@ -228,7 +223,7 @@ |
105 | # user does not have gnome-session running or isn't logged in at |
106 | # all |
107 | log.msg("No gnome-session environment for user %s" % username) |
108 | - self.parent.scripts_state[uid] = NOT_RUNNING #unmarking to avoid dead-lock |
109 | + self.scripts_state = NOT_RUNNING #unmarking to avoid dead-lock |
110 | return |
111 | fp.close() |
112 | |
113 | @@ -257,14 +252,17 @@ |
114 | # XXX all parent calls should be refactored out of the AsyncAPI class |
115 | # to keep the code cleaner and the logic more limited to one particular |
116 | # task |
117 | - accoms = self.parent.list_unlocked_not_completed() |
118 | - |
119 | - totalscripts = len(accoms) |
120 | - log.msg("Need to run (%d) scripts:" % totalscripts) |
121 | - log.msg(str(accoms)) |
122 | + |
123 | + queuesize = len(self.parent.scripts_queue) |
124 | + |
125 | + log.msg("--- Starting Running Scripts - %d items on the queue ---" % (queuesize)) |
126 | + timestart = time.time() |
127 | + self.parent.service.scriptrunner_start() |
128 | |
129 | - scriptcount = 1 |
130 | - for accomID in accoms: |
131 | + while queuesize > 0: |
132 | + accomID = self.parent.scripts_queue.popleft() |
133 | + log.msg("Running %s, left on queue: %d" % (accomID, queuesize-1)) |
134 | + |
135 | # First ensure that the acccomplishemt has not yet completed. |
136 | # It happens that the .asc file is present, but we miss the |
137 | # signal it triggers - so here we can re-check if it is not |
138 | @@ -272,15 +270,15 @@ |
139 | if self.parent._check_if_acc_is_completed(accomID): |
140 | self.parent.accomplish(accomID) |
141 | continue |
142 | - |
143 | # Run the acc script and determine exit code. |
144 | scriptpath = self.parent.get_acc_script_path(accomID) |
145 | - msg = "%s/%s: %s" % (scriptcount, totalscripts, scriptpath) |
146 | - log.msg(msg) |
147 | + if scriptpath is None: |
148 | + log.msg("...No script for this accomplishment, skipping") |
149 | + continue |
150 | exitcode = yield self.run_a_subprocess([scriptpath]) |
151 | if exitcode == 0: |
152 | - self.parent.accomplish(accomID) |
153 | log.msg("...Accomplished") |
154 | + self.parent.accomplish(accomID) |
155 | elif exitcode == 1: |
156 | log.msg("...Not Accomplished") |
157 | elif exitcode == 2: |
158 | @@ -289,8 +287,14 @@ |
159 | log.msg("...Could not get extra-information") |
160 | else: |
161 | log.msg("...Error code %d" % exitcode) |
162 | - scriptcount = scriptcount + 1 |
163 | + |
164 | + # New queue size is determined on the very end, since accomplish() |
165 | + # might have added something new to the queue. |
166 | + queuesize = len(self.parent.scripts_queue) |
167 | |
168 | + |
169 | + log.msg("The queue is now empty - stopping the scriptrunner.") |
170 | + |
171 | os.environ = oldenviron |
172 | |
173 | # XXX eventually the code in this method will be rewritten using |
174 | @@ -300,18 +304,10 @@ |
175 | timefinal = round((timeend - timestart), 2) |
176 | |
177 | log.msg( |
178 | - "--- Completed Running Scripts in %.2f seconds---" % timefinal) |
179 | + "--- Emptied the scripts queue in %.2f seconds---" % timefinal) |
180 | self.parent.service.scriptrunner_finish() |
181 | - |
182 | - # checking whether this function was called while script execution was in progress... |
183 | - rerun = (self.parent.scripts_state[uid] is NEEDS_RE_RUNNING) |
184 | - # unsetting the lock |
185 | - self.parent.scripts_state[uid] = NOT_RUNNING |
186 | - # re-running scripts if needed |
187 | - if rerun: |
188 | - log.msg("Re-running scripts as intended...") |
189 | - self.run_scripts_for_user(uid) |
190 | |
191 | + self.scripts_state = NOT_RUNNING |
192 | |
193 | class Accomplishments(object): |
194 | """The main accomplishments daemon. |
195 | @@ -334,24 +330,7 @@ |
196 | self.service = service |
197 | self.asyncapi = AsyncAPI(self) |
198 | |
199 | - # The following dictionary represents state of scripts. |
200 | - # It's a dictionary and not a single variable, because scripts may be run |
201 | - # for each user independently. For each user, the state can be either |
202 | - # RUNNING, NOT_RUNNING or NEEDS_RE_RUNNING. If an entry for a |
203 | - # particular UID does not exist, it should be treated as NOT_RUNNING. |
204 | - # The use of this flag is to aviod running several instances of |
205 | - # run_scripts_for_user, which might result in undefined, troubleful |
206 | - # behavior. The flags are NOT_RUNNING by default, and are set to |
207 | - # RUNNING when the run_scripts_for_user starts. However, if it has been |
208 | - # already set to RUNNING, the function will abort, and will instead set the |
209 | - # flag to NEEDS_RE_RUNNING, in order to mark that the scripts have to |
210 | - # be run once more, because something might have changed since we run |
211 | - # them the last time (as the run_scripts_for_user was called while |
212 | - # scripts were being executed). Setting the flag to NEEDS_RE_RUNNING |
213 | - # will cause run_scripts_for_user to redo everything after having |
214 | - # finished it's current task in progress. Otherwise it will eventually |
215 | - # set the flag back to NOT_RUNNING. |
216 | - self.scripts_state = {} |
217 | + self.scripts_queue = deque() |
218 | |
219 | # create config / data dirs if they don't exist |
220 | self.dir_config = os.path.join( |
221 | @@ -387,22 +366,18 @@ |
222 | |
223 | self.reload_accom_database() |
224 | |
225 | - # XXX this wait-until thing should go away; it should be replaced by a |
226 | - # deferred-returning function that has a callback which fires off |
227 | - # generate_all_trophis and schedule_run_scripts... |
228 | - #self._wait_until_a_sig_file_arrives() |
229 | self.sd.connect_signal("DownloadFinished", self._process_recieved_asc_file) |
230 | self._create_all_trophy_icons() |
231 | |
232 | def get_media_file(self, media_file_name): |
233 | - log.msg("MEDIA_FILE_NAME:") |
234 | - log.msg(media_file_name) |
235 | - log.msg("MEDIA_DIR:") |
236 | - log.msg(media_dir) |
237 | + #log.msg("MEDIA_FILE_NAME:") |
238 | + #log.msg(media_file_name) |
239 | + #log.msg("MEDIA_DIR:") |
240 | + #log.msg(media_dir) |
241 | #media_filename = get_data_file(media_dir.split, '%s' % (media_file_name,)) |
242 | media_filename = os.path.join(media_dir, media_file_name) |
243 | - log.msg("MEDIA_FILENAME:") |
244 | - log.msg(media_filename) |
245 | + #log.msg("MEDIA_FILENAME:") |
246 | + #log.msg(media_filename) |
247 | |
248 | if not os.path.exists(media_filename): |
249 | media_filename = None |
250 | @@ -730,21 +705,6 @@ |
251 | |
252 | return result |
253 | |
254 | - def run_scripts_for_all_active_users(self): |
255 | - for uid in [x.pw_uid for x in pwd.getpwall() |
256 | - if x.pw_dir.startswith('/home/') and x.pw_shell != '/bin/false']: |
257 | - os.seteuid(0) |
258 | - self.asyncapi.run_scripts_for_user(uid) |
259 | - |
260 | - def run_scripts(self, run_by_client): |
261 | - uid = os.getuid() |
262 | - if uid == 0: |
263 | - log.msg("Run scripts for all active users") |
264 | - self.run_scripts_for_all_active_users() |
265 | - else: |
266 | - log.msg("Run scripts for user") |
267 | - self.asyncapi.run_scripts_for_user(uid) |
268 | - |
269 | def create_extra_information_file(self, item, data): |
270 | """Does exactly the same as write_extra_information_file(), but it does not |
271 | overwrite any existing data""" |
272 | @@ -779,9 +739,9 @@ |
273 | self.service.trophy_received(accomID) |
274 | self._display_accomplished_bubble(accomID) |
275 | self._display_unlocked_bubble(accomID) |
276 | - self._mark_as_completed(accomID) |
277 | - |
278 | - self.run_scripts(0) |
279 | + # Mark as completed and get list of new opportunities |
280 | + just_unlocked = self._mark_as_completed(accomID) |
281 | + self.run_scripts(just_unlocked) |
282 | |
283 | def write_extra_information_file(self, item, data): |
284 | log.msg( |
285 | @@ -1076,7 +1036,11 @@ |
286 | return self.accDB[accomID]['completed'] |
287 | |
288 | def get_acc_script_path(self,accomID): |
289 | - return self.accDB[accomID]['script-path'] |
290 | + res = self.accDB[accomID]['script-path'] |
291 | + if not os.path.exists(res): |
292 | + return None |
293 | + else: |
294 | + return res |
295 | |
296 | def get_acc_icon(self,accomID): |
297 | return self.accDB[accomID]['icon'] |
298 | @@ -1153,6 +1117,29 @@ |
299 | |
300 | def list_collections(self): |
301 | return [col for col in self.accDB if self.accDB[col]['type'] == 'collection'] |
302 | + |
303 | + # ====== Scriptrunner functions ====== |
304 | + |
305 | + def run_script(self,accomID): |
306 | + if not self.get_acc_exists(accomID): |
307 | + return |
308 | + self.run_scripts([accomID]) |
309 | + |
310 | + def run_scripts(self,which=None): |
311 | + if which == None: |
312 | + to_schedule = self.list_unlocked_not_completed() |
313 | + elif type(which) is int or type(which) is bool or type(which) is dbus.Boolean: |
314 | + log.msg("Note: This call to run_scripts is incorrect, run_scripts no more takes an int as an argument (it takes - optionally - a list of accomID to run their scripts)") |
315 | + to_schedule = self.list_unlocked_not_completed() |
316 | + else: |
317 | + if len(which) is 0: # am empty list |
318 | + return |
319 | + to_schedule = which |
320 | + log.msg("Adding to scripts queue: %s " % (str(to_schedule))) |
321 | + for i in to_schedule: |
322 | + if not i in self.scripts_queue: |
323 | + self.scripts_queue.append(i) |
324 | + self.asyncapi.start_scriptrunner() |
325 | |
326 | # ====== Viewer-specific functions ====== |
327 | |
328 | @@ -1239,8 +1226,9 @@ |
329 | self.service.trophy_received(accomID) |
330 | self._display_accomplished_bubble(accomID) |
331 | self._display_unlocked_bubble(accomID) |
332 | - self._mark_as_completed(accomID) |
333 | - self.run_scripts(0) |
334 | + # Mark as completed and get list of new opportunities |
335 | + just_unlocked = self._mark_as_completed(accomID) |
336 | + self.run_scripts(just_unlocked) |
337 | |
338 | return True |
339 | |
340 | @@ -1424,10 +1412,18 @@ |
341 | return config.get("trophy", "date-accomplished") |
342 | |
343 | def _mark_as_completed(self,accomID): |
344 | + # Marks accomplishments as completed int the accDB, and returns a list |
345 | + # of accomIDs that just got unlocked. |
346 | self.accDB[accomID]['completed'] = True |
347 | accs = self.list_depending_on(accomID) |
348 | + res = [] |
349 | for acc in accs: |
350 | + before = self.accDB[acc]['locked'] |
351 | self.accDB[acc]['locked'] = self._check_if_acc_is_locked(acc) |
352 | + # If it just got unlocked... |
353 | + if (before == True and self.accDB[acc]['locked'] == False): |
354 | + res.append(acc) |
355 | + return res |
356 | |
357 | #Other significant system functions |
358 | def get_API_version(self): |
359 | |
360 | === modified file 'accomplishments/daemon/dbusapi.py' |
361 | --- accomplishments/daemon/dbusapi.py 2012-06-25 22:34:30 +0000 |
362 | +++ accomplishments/daemon/dbusapi.py 2012-07-09 17:35:12 +0000 |
363 | @@ -79,9 +79,14 @@ |
364 | return self.api.get_all_extra_information() |
365 | |
366 | @dbus.service.method(dbus_interface='org.ubuntu.accomplishments', |
367 | - in_signature="b", out_signature="") |
368 | - def run_scripts(self, run_by_client): |
369 | - return self.api.run_scripts(run_by_client) |
370 | + in_signature="v", out_signature="") |
371 | + def run_scripts(self, accomIDlist=None): |
372 | + return self.api.run_scripts(accomIDlist) |
373 | + |
374 | + @dbus.service.method(dbus_interface='org.ubuntu.accomplishments', |
375 | + in_signature="s", out_signature="") |
376 | + def run_script(self, accomID): |
377 | + return self.api.run_script(accomID) |
378 | |
379 | @dbus.service.method(dbus_interface='org.ubuntu.accomplishments', |
380 | in_signature="ss", out_signature="aa{sv}") |
381 | |
382 | === modified file 'accomplishments/daemon/service.py' |
383 | --- accomplishments/daemon/service.py 2012-04-12 00:20:10 +0000 |
384 | +++ accomplishments/daemon/service.py 2012-07-09 17:35:12 +0000 |
385 | @@ -69,7 +69,7 @@ |
386 | intertal. |
387 | """ |
388 | def __init__(self, interval, api): |
389 | - TimerService.__init__(self, interval, api.run_scripts, False) |
390 | + TimerService.__init__(self, interval, api.run_scripts) |
391 | |
392 | def startService(self): |
393 | log.msg("Starting up script runner service ...") |
394 | |
395 | === modified file 'po/accomplishments-daemon.pot' |
396 | --- po/accomplishments-daemon.pot 2012-05-24 13:21:09 +0000 |
397 | +++ po/accomplishments-daemon.pot 2012-07-09 17:35:12 +0000 |
398 | @@ -8,7 +8,7 @@ |
399 | msgstr "" |
400 | "Project-Id-Version: PACKAGE VERSION\n" |
401 | "Report-Msgid-Bugs-To: \n" |
402 | -"POT-Creation-Date: 2012-05-24 15:18+0200\n" |
403 | +"POT-Creation-Date: 2012-07-09 16:22+0200\n" |
404 | "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" |
405 | "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" |
406 | "Language-Team: LANGUAGE <LL@li.org>\n" |
407 | @@ -25,15 +25,15 @@ |
408 | msgid "Clear your trophies collection" |
409 | msgstr "" |
410 | |
411 | -#: ../accomplishments/daemon/api.py:1246 |
412 | +#: ../accomplishments/daemon/api.py:1336 |
413 | msgid "You have accomplished something!" |
414 | msgstr "" |
415 | |
416 | -#: ../accomplishments/daemon/api.py:1257 |
417 | +#: ../accomplishments/daemon/api.py:1347 |
418 | #, python-format |
419 | msgid "You have unlocked %s new opportunity." |
420 | msgstr "" |
421 | |
422 | -#: ../accomplishments/daemon/api.py:1259 |
423 | +#: ../accomplishments/daemon/api.py:1349 |
424 | msgid "Opportunities Unlocked!" |
425 | msgstr "" |