Merge lp:~mvo/software-center/recommender_uuid_consolidation into lp:software-center

Proposed by Michael Vogt on 2012-03-01
Status: Rejected
Rejected by: Michael Vogt on 2012-04-02
Proposed branch: lp:~mvo/software-center/recommender_uuid_consolidation
Merge into: lp:software-center
Diff against target: 169 lines (+51/-39)
4 files modified
softwarecenter/backend/recagent.py (+31/-13)
softwarecenter/ui/gtk3/app.py (+0/-26)
softwarecenter/ui/gtk3/widgets/recommendations.py (+7/-0)
test/test_recagent.py (+13/-0)
To merge this branch: bzr merge lp:~mvo/software-center/recommender_uuid_consolidation
Reviewer Review Type Date Requested Status
Gary Lasker (community) 2012-03-01 Needs Fixing on 2012-03-01
Review via email: mp+95322@code.launchpad.net

Description of the change

This branch does some cleanup in the way we store the recommender_uuid.

All usage from app.py is removed as its currently not used or needed there. We only use it to write it in the config
but that can be done in the recagent.py as well.

The RecommenderAgent.recommender_uuid is a property to ensure its always in sync with the (global singelton)
SoftwareCenterConfig. This ensures that on close SoftwareCenterConfig.write() will include the recommender_uuid
value.

Plus a test to ensure that the property is calling the SoftwareCenterConfig in the expected way.

To post a comment you must log in.
Gary Lasker (gary-lasker) wrote :
Download full text (4.3 KiB)

Hi mvo, thanks for this! I think it's a good idea to consolidate the UUID stuff more, however, there are two reasons why I left the connection to app.py (with the signal). The first is because I planned to add an opt-out menu item (which will be a dead-simple change). The second is that I thought it best to keep all of the code that writes to the main config file in one place, that is, in app.py. Else somebody else will have to search if they want to explain where this mysterious other field is coming from in the main config file.

In any case, I am seeing two problems with it the current branch. One is a failure in the unit test, the other occurs when running the app itself (specifically, soemthing that would occur during a first run in a new install, further details below).

First, the unit test:

======================================================================
ERROR: test_mocked_recagent_post_submit_profile (__main__.TestRecommenderAgent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_recagent.py", line 48, in test_mocked_recagent_post_submit_profile
    recommender_agent.post_submit_profile(db)
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/backend/recagent.py", line 98, in post_submit_profile
    self.recommender_uuid = get_uuid()
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/backend/recagent.py", line 236, in _set_recommender_uuid
    config.set("general", "recommender_uuid", self.recommender_uuid)
  File "/usr/lib/python2.7/ConfigParser.py", line 753, in set
    ConfigParser.set(self, section, option, value)
  File "/usr/lib/python2.7/ConfigParser.py", line 396, in set
    raise NoSectionError(section)
NoSectionError: No section: 'general'

======================================================================
FAIL: test_recagent_recommender_uuid_property (__main__.TestRecommenderAgent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_recagent.py", line 161, in test_recagent_recommender_uuid_property
    self.assertEqual(recommender_agent.recommender_uuid, "foxy-uuid")
AssertionError: '' != 'foxy-uuid'

----------------------------------------------------------------------
Ran 7 tests in 6.296s

FAILED (failures=1, errors=1)

---

And the failure with the app itself occurs in the case where we have not yet made the cfg file (you can see this with a 'rm .config/software-center/softwarecenter.cfg'). This would happen on first run of software-center for a new install, for instance, and if the user opt-in to recommendations then. What happens in the UI is that the "Uploading user profile" spinner hangs.

Traceback (most recent call last):
  File "/home/tremolux/Projects/precise/software-center_review_mvo_recommender_uuid_consolidation/software-center/softwarecenter/ui/gtk3/widgets/recommendations.py", line 127, in _on_opt_in_button_clicked
    self._upload_user_profile_and_get_recommendations()
  File "/home/tremolux/Projects/precis...

Read more...

review: Needs Fixing
Michael Vogt (mvo) wrote :

I set this to rejected as it no longer applies with the recent changes.

Unmerged revisions

2799. By Michael Vogt on 2012-03-01

REFACTOR: remove recommender_uuid from app.py as it does not need to know about this (the widget is self contained currently and the state can set set using the global get_config() config that will saved on s-c exit); also make recommender_uuid a property so that reads/writes are automatically stored in the global s-c config

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/backend/recagent.py'
2--- softwarecenter/backend/recagent.py 2012-02-28 20:27:50 +0000
3+++ softwarecenter/backend/recagent.py 2012-03-01 09:12:23 +0000
4@@ -75,8 +75,8 @@
5 def __init__(self, xid=None):
6 GObject.GObject.__init__(self)
7 self.xid = xid
8- self.recommender_uuid = self._get_recommender_uuid()
9
10+ # methods
11 def query_server_status(self):
12 # build the command
13 spawner = SpawnHelper()
14@@ -202,18 +202,6 @@
15 def _on_recommend_top_data(self, spawner, piston_top_apps):
16 self.emit("recommend-top", piston_top_apps)
17
18- def _get_recommender_uuid(self):
19- """ returns the recommender UUID value, which can be empty if it
20- has not yet been set (indicating that the user has not yet
21- opted-in to the recommender service)
22- """
23- config = get_config()
24- if config.has_option("general", "recommender_uuid"):
25- recommender_uuid = config.get("general", "recommender_uuid")
26- if recommender_uuid:
27- return recommender_uuid
28- return ""
29-
30 def _generate_submit_profile_data(self, recommender_uuid, package_list):
31 submit_profile_data = [
32 {
33@@ -223,6 +211,36 @@
34 ]
35 return submit_profile_data
36
37+ # properties
38+ def _get_recommender_uuid(self):
39+ """ returns the recommender UUID value, which can be empty if it
40+ has not yet been set (indicating that the user has not yet
41+ opted-in to the recommender service)
42+ """
43+ if not hasattr(self, "_recommender_uuid"):
44+ config = get_config()
45+ if config.has_option("general", "recommender_uuid"):
46+ self._recommender_uuid = config.get(
47+ "general", "recommender_uuid")
48+ else:
49+ self._recommender_uuid = ""
50+ return self._recommender_uuid
51+
52+ return self._recommender_uuid
53+ def _set_recommender_uuid(self, value):
54+ """ this sets the recommender UUID value, both internally for
55+ this class and in the app wide software-center config
56+ """
57+ self._recommender_uuid = value
58+ config = get_config()
59+ config.set("general", "recommender_uuid", self.recommender_uuid)
60+ recommender_uuid = property(
61+ _get_recommender_uuid, _set_recommender_uuid, None,
62+ """ The (persistent) recommender_uuid property that is stored
63+ in the config under general/recommender_uuid
64+ """)
65+
66+
67
68 if __name__ == "__main__":
69 from gi.repository import Gtk
70
71=== modified file 'softwarecenter/ui/gtk3/app.py'
72--- softwarecenter/ui/gtk3/app.py 2012-02-29 17:23:24 +0000
73+++ softwarecenter/ui/gtk3/app.py 2012-03-01 09:12:23 +0000
74@@ -258,7 +258,6 @@
75 self.scagent = None
76 self.sso = None
77 self.available_for_me_query = None
78- self.recommender_uuid = ""
79
80 Gtk.Window.set_default_icon_name("softwarecenter")
81
82@@ -468,25 +467,6 @@
83
84 def on_available_pane_created(self, widget):
85 self.available_pane.searchentry.grab_focus()
86- # connect a signal to monitor the recommendations opt-in state and
87- # persist the recommendations uuid on an opt-in
88- self.available_pane.cat_view.recommended_for_you_panel.connect(
89- "recommendations-opt-in",
90- self._on_recommendations_opt_in)
91- self.available_pane.cat_view.recommended_for_you_panel.connect(
92- "recommendations-opt-out",
93- self._on_recommendations_opt_out)
94-
95- #~ def on_installed_pane_created(self, widget):
96- #~ pass
97-
98- def _on_recommendations_opt_in(self, agent, recommender_uuid):
99- self.recommender_uuid = recommender_uuid
100-
101- def _on_recommendations_opt_out(self):
102- # if the user opts back out of the recommender service, we
103- # reset the UUID to indicate it
104- self.recommender_uuid = ""
105
106 def _on_update_software_center_agent_finished(self, pid, condition):
107 LOG.info("software-center-agent finished with status %i" % os.WEXITSTATUS(condition))
108@@ -1231,9 +1211,6 @@
109 else:
110 # initial default state is to add to launcher, per spec
111 self.available_pane.add_to_launcher_enabled = True
112- if self.config.has_option("general", "recommender_uuid"):
113- self.recommender_uuid = self.config.get("general",
114- "recommender_uuid")
115
116 def save_state(self):
117 LOG.debug("save_state")
118@@ -1255,9 +1232,6 @@
119 self.config.set("general", "add_to_launcher", "True")
120 else:
121 self.config.set("general", "add_to_launcher", "False")
122- self.config.set("general",
123- "recommender_uuid",
124- self.recommender_uuid)
125 self.config.write()
126
127 def run(self, args):
128
129=== modified file 'softwarecenter/ui/gtk3/widgets/recommendations.py'
130--- softwarecenter/ui/gtk3/widgets/recommendations.py 2012-02-29 19:58:05 +0000
131+++ softwarecenter/ui/gtk3/widgets/recommendations.py 2012-03-01 09:12:23 +0000
132@@ -110,6 +110,13 @@
133 label.set_alignment(0, 0.5)
134 label.set_line_wrap(True)
135 vbox.pack_start(label, False, False, 0)
136+
137+ # FIXME: there is no opt-out button yet!
138+ def _on_opt_out_button_clicked(self, button):
139+ # if the user opts back out of the recommender service, we
140+ # reset the UUID to indicate it
141+ self.recommender_agent.recommender_uuid = ""
142+ self.emit("recommendations-opt-out")
143
144 def _on_opt_in_button_clicked(self, button):
145 # we upload the user profile here, and only after this is finished
146
147=== modified file 'test/test_recagent.py'
148--- test/test_recagent.py 2012-03-01 00:14:51 +0000
149+++ test/test_recagent.py 2012-03-01 09:12:23 +0000
150@@ -152,6 +152,19 @@
151 self.loop.run()
152 self.assertTrue(self.error)
153
154+ @patch('softwarecenter.config.SoftwareCenterConfig')
155+ def test_recagent_recommender_uuid_property(self, mock_config_klass):
156+ mock_config = mock_config_klass.return_value
157+ mock_config.has_option.return_value = True
158+ mock_config.get.return_value = "foxy-uuid"
159+ recommender_agent = RecommenderAgent()
160+ self.assertEqual(recommender_agent.recommender_uuid, "foxy-uuid")
161+ recommender_agent.recommender_uuid = "new-value"
162+ self.assertEqual(recommender_agent.recommender_uuid, "new-value")
163+ self.assertTrue(mock_config.set.called)
164+ mock_config.set.assertCalledWith("general","recommender_uuid",
165+ "new-value")
166+
167 if __name__ == "__main__":
168 #import logging
169 #logging.basicConfig(level=logging.DEBUG)

Subscribers

People subscribed via source and target branches