GTG

Merge lp:~huxuan/gtg/bug-907676 into lp:~gtg/gtg/old-trunk

Proposed by Xuan (Sean) Hu
Status: Rejected
Rejected by: Izidor Matušov
Proposed branch: lp:~huxuan/gtg/bug-907676
Merge into: lp:~gtg/gtg/old-trunk
Diff against target: 97 lines (+30/-7)
2 files modified
GTG/backends/backend_localfile.py (+28/-7)
GTG/tests/test_backends.py (+2/-0)
To merge this branch: bzr merge lp:~huxuan/gtg/bug-907676
Reviewer Review Type Date Requested Status
Izidor Matušov Disapprove
Xuan (Sean) Hu (community) Needs Resubmitting
Review via email: mp+103529@code.launchpad.net

Description of the change

It's a trial to solve bug #907676.
Welcome to any suggestions or criticism.
Feel free to comment your opinion down

To post a comment you must log in.
lp:~huxuan/gtg/bug-907676 updated
1177. By Xuan (Sean) Hu

backend_localfile.py

1) remove test code. (forgive silly me)

1178. By Xuan (Sean) Hu

backend_localfile.py

1) a little code revise to be more pylint.
Though there exist more.

Revision history for this message
Izidor Matušov (izidor) wrote :

Your solution looks good and should work in the most of the situation. You might not get right this kind of situation:
" You get many requests in 20 seconds. You filter them because it is in burst. But then you have no save requests for long time like 30 minutes. Afterwards GTG crashes or user turn off the computer without closing GTG - the changes in the first 20 seconds are lost. "

Do you see that usecase? I propose to solve this bug by creating a timer which save XML and then timer is deleted. If the timer exists, save action is thrown away (it is already going to be saved). You might need to add locks to prevent modification of XML while saving XML.

review: Needs Fixing (code)
Revision history for this message
Bertrand Rousseau (bertrand-rousseau) wrote :

I don't think we'd like to keep that particular line ;-)

+ print 'fuck'

2012/4/26 Izidor Matušov <email address hidden>:
> Review: Needs Fixing code
>
> Your solution looks good and should work in the most of the situation. You might not get right this kind of situation:
> " You get many requests in 20 seconds. You filter them because it is in burst. But then you have no save requests for long time like 30 minutes. Afterwards GTG crashes or user turn off the computer without closing GTG - the changes in the first 20 seconds are lost. "
>
> Do you see that usecase? I propose to solve this bug by creating a timer which save XML and then timer is deleted. If the timer exists, save action is thrown away (it is already going to be saved). You might need to add locks to prevent modification of XML while saving XML.
> --
> https://code.launchpad.net/~huxuan/gtg/bug-907676/+merge/103529
> You are subscribed to branch lp:gtg.

--
Bertrand Rousseau

Revision history for this message
Xuan (Sean) Hu (huxuan) wrote :

izidor:
Got that, I will try to implement it.

Bertrand Rousseau:
Really sorry for the testing code. I did have delete it.

lp:~huxuan/gtg/bug-907676 updated
1179. By Xuan (Sean) Hu

backend_localfile.py

1) Make timer for saving

Revision history for this message
Xuan (Sean) Hu (huxuan) wrote :

hi, I have resubmit and wish your more suggestion.

Revision history for this message
Xuan (Sean) Hu (huxuan) :
review: Needs Resubmitting
lp:~huxuan/gtg/bug-907676 updated
1180. By Xuan (Sean) Hu

backend_localfile.py

1) revised for silly logical error

1181. By Xuan (Sean) Hu

backend_localfile.py

1) silly repo error (thanks to pylint)
2) docstring revise

1182. By Xuan (Sean) Hu

backend_localfile.py

1) silly logical error.

1183. By Xuan (Sean) Hu

backend_localfile.py

1) bug fix for error indent

1184. By Xuan (Sean) Hu

backend_localfile.py

1) remove self.timer_exist for simplier code

1185. By Xuan (Sean) Hu

Merge from lp:gtg

Revision history for this message
Izidor Matušov (izidor) wrote :

Could you explain me why do you need self.timer_backup?

The only way how to call savexml from set_task & remove_task should be through a timer. In other words, there can't savexml in set_task & remove_task methods.

Please, decrease Timer time to shorter interval like 1 or 2 seconds. (This should prevent bursts of savexml calls but still saving enough and not lose data)

If something is not clear, feel free to contact me directly :)

review: Needs Fixing (code)
lp:~huxuan/gtg/bug-907676 updated
1186. By Xuan (Sean) Hu

merge from lp:gtg

1187. By Xuan (Sean) Hu

revise according to izidor's comment:

1) backend_localfile.py: remove redundant savexml, reduce timer interval
2) test_backends.py: add time.sleep to wait timer so as to pass the test

1188. By Xuan (Sean) Hu

merge from lp:gtg

Revision history for this message
Xuan (Sean) Hu (huxuan) wrote :

izidor, hi

Sorry for reply so late. I'm just back from thesis season with everything almost done.

I just use self.time_backup as a sign of whether to create backup localfile. If remove that then how should we judge whether savexml should just save with rewrite or as a backup?
I have remove the redundant savexml in set_task and remove_task, but find if fail in the test caused by the timer. I add a time.sleep so as to make it pass. just note it here for attention.
The code still need some test and revise. I will try to get you in the irc channel.

lp:~huxuan/gtg/bug-907676 updated
1189. By Xuan (Sean) Hu

make sleep with 2 sec to avoid potential make check error

Revision history for this message
Izidor Matušov (izidor) wrote :

As I described in the bug and on IRC, the bug should be solved by changing internal interface. Closing as "rejected". Nothing personal :)

review: Disapprove

Unmerged revisions

1189. By Xuan (Sean) Hu

make sleep with 2 sec to avoid potential make check error

1188. By Xuan (Sean) Hu

merge from lp:gtg

1187. By Xuan (Sean) Hu

revise according to izidor's comment:

1) backend_localfile.py: remove redundant savexml, reduce timer interval
2) test_backends.py: add time.sleep to wait timer so as to pass the test

1186. By Xuan (Sean) Hu

merge from lp:gtg

1185. By Xuan (Sean) Hu

Merge from lp:gtg

1184. By Xuan (Sean) Hu

backend_localfile.py

1) remove self.timer_exist for simplier code

1183. By Xuan (Sean) Hu

backend_localfile.py

1) bug fix for error indent

1182. By Xuan (Sean) Hu

backend_localfile.py

1) silly logical error.

1181. By Xuan (Sean) Hu

backend_localfile.py

1) silly repo error (thanks to pylint)
2) docstring revise

1180. By Xuan (Sean) Hu

backend_localfile.py

1) revised for silly logical error

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'GTG/backends/backend_localfile.py'
2--- GTG/backends/backend_localfile.py 2012-03-05 15:23:05 +0000
3+++ GTG/backends/backend_localfile.py 2012-05-30 08:35:23 +0000
4@@ -27,6 +27,7 @@
5
6 import os
7 import uuid
8+import threading
9
10 from GTG import _
11 from GTG.backends.genericbackend import GenericBackend
12@@ -50,8 +51,8 @@
13 # default path for filenames
14 DEFAULT_PATH = CoreConfig().get_data_dir()
15
16- # General description of the backend: these are used to show a description of
17- # the backend to the user when s/he is considering adding it.
18+ # General description of the backend: these are used to show a description
19+ # of the backend to the user when s/he is considering adding it.
20 # BACKEND_NAME is the name of the backend used internally (it must be
21 # unique).
22 # Please note that BACKEND_NAME and BACKEND_ICON_NAME should *not* be
23@@ -111,6 +112,22 @@
24 # Make safety daily backup after loading
25 cleanxml.savexml(self._parameters["path"], self.doc, backup=True)
26
27+ # Set timer as None and mark it not backup
28+ self.timer = None
29+ self.timer_backup = False
30+
31+ def timer_end(self):
32+ """ Save and set timer as None and mark it not backup """
33+ cleanxml.savexml(self._parameters["path"], self.doc, \
34+ backup=self.timer_backup)
35+ self.timer = None
36+ self.timer_backup = False
37+
38+ def timer_start(self):
39+ """ Start timer for saving """
40+ self.timer = threading.Timer(1, self.timer_end)
41+ self.timer.start()
42+
43 def initialize(self):
44 """ This is called when a backend is enabled """
45 super(Backend, self).initialize()
46@@ -131,8 +148,8 @@
47 self._parameters["path"], "project")
48
49 def start_get_tasks(self):
50- """ This function starts submitting the tasks from the XML file into GTG core.
51- It's run as a separate thread.
52+ """ This function starts submitting the tasks from the XML file into
53+ GTG core. It's run as a separate thread.
54
55 @return: start_get_tasks() might not return or finish
56 """
57@@ -178,8 +195,9 @@
58 modified = True
59
60 #if the XML object has changed, we save it to file
61- if modified and self._parameters["path"] and self.doc :
62- cleanxml.savexml(self._parameters["path"], self.doc)
63+ if modified and self._parameters["path"] and self.doc \
64+ and not self.timer:
65+ self.timer_start()
66
67 def remove_task(self, tid):
68 """ This function is called from GTG core whenever a task must be
69@@ -195,4 +213,7 @@
70
71 #We save the XML file only if it's necessary
72 if modified:
73- cleanxml.savexml(self._parameters["path"], self.doc, backup=True)
74+ if self.timer:
75+ self.timer_backup = True
76+ else:
77+ self.timer_start()
78
79=== modified file 'GTG/tests/test_backends.py'
80--- GTG/tests/test_backends.py 2012-03-05 15:23:05 +0000
81+++ GTG/tests/test_backends.py 2012-05-30 08:35:23 +0000
82@@ -27,6 +27,7 @@
83 import unittest
84 import os
85 import xdg
86+import time
87
88 # GTG imports
89 from GTG.backends import backend_localfile as localfile
90@@ -100,6 +101,7 @@
91 expectedres = True
92 beobj.remove_task("0@1")
93 beobj.quit()
94+ time.sleep(2)
95 dataline = open(self.taskpath, 'r').read()
96 if "0@1" in dataline:
97 res = False

Subscribers

People subscribed via source and target branches

to status/vote changes: