Merge lp:~james-w/launchpad-work-items-tracker/multi-project into lp:launchpad-work-items-tracker

Proposed by James Westby on 2010-12-09
Status: Merged
Merged at revision: 268
Proposed branch: lp:~james-w/launchpad-work-items-tracker/multi-project
Merge into: lp:launchpad-work-items-tracker
Diff against target: 145 lines (+49/-19)
3 files modified
burndown-chart (+2/-0)
collect (+42/-16)
report_tools.py (+5/-3)
To merge this branch: bzr merge lp:~james-w/launchpad-work-items-tracker/multi-project
Reviewer Review Type Date Requested Status
Martin Pitt (community) 2010-12-09 Needs Fixing on 2011-02-09
Review via email: mp+43221@code.launchpad.net

Description of the Change

Hi,

Apologies for the bug in the previous change, I hadn't tested with
a project base, just Ubuntu.

This is the proper fix that passes the LP object for both calls, which
will make it work in both cases.

Thanks,

James

To post a comment you must log in.
Martin Pitt (pitti) wrote :

I reverted the multi-projects commit for now, as it wreaked havoc. It causes the ubuntu database to only contain the linaro milestones, and thus breaking all the milestone charts and sending tons of errors to everyone.

What is the rationale here in the first place? linaro has separate projects in Launchpad, thus we should also have separate WI reports/burndowns, such as

  http://people.canonical.com/~platform/workitems/linaro-pm-wg/all.html
  http://people.canonical.com/~platform/workitems/linaro-toolchain-wg/all.html

(you also use a different burndown chart style). Trying to merge different projects with different milestones into one set of charts will just create confusion. Either you need to ignore the milestones from either side (which is obvioulsy not acceptable), or you need to merge the milestones from all projects. But this will create a combinatorial explosion of charts, and also break the automatic reset of per-milestone burndown charts, so it's also not feasible.

So let's discuss that a bit further first.

James Westby (james-w) wrote :

> I reverted the multi-projects commit for now, as it wreaked havoc. It causes
> the ubuntu database to only contain the linaro milestones, and thus breaking
> all the milestone charts and sending tons of errors to everyone.

Sorry about that.

> What is the rationale here in the first place? linaro has separate projects in
> Launchpad, thus we should also have separate WI reports/burndowns, such as
>
> http://people.canonical.com/~platform/workitems/linaro-pm-wg/all.html
> http://people.canonical.com/~platform/workitems/linaro-toolchain-wg/all.html

We have changed where we put blueprints somewhat this cycle. For instance, the linaro
foundations team has blueprints against at least "ubuntu", and "linaro", and they
don't want to have to try and reconcile two burndown charts in their head.

> (you also use a different burndown chart style). Trying to merge different
> projects with different milestones into one set of charts will just create
> confusion. Either you need to ignore the milestones from either side (which is
> obvioulsy not acceptable), or you need to merge the milestones from all
> projects.

As the milestones are just name->date mappings I don't think that storing all of
them is a problem. I can arrange to have ubuntu's milestones take precedence, so
we can't do something like have a natty-alpha-2 with a different date.

> But this will create a combinatorial explosion of charts,

Because you generate a chart for every milestone? Would it be enough not to generate
empty charts?

> and also
> break the automatic reset of per-milestone burndown charts, so it's also not
> feasible.

I don't follow this part, can you point me to the auto-reset code so that I can
see what will break please?

> So let's discuss that a bit further first.

Sure, I'm happy to do that, but this is an urgent matter for Linaro, so I'm willing
and keen to make the changes to get this landed ASAP. If it's not acceptable in
Ubuntu's instance, then I imagine we will end up with our own instance with changes
similar to these.

Thanks,

James

246. By James Westby on 2010-12-10

Make sure we import milestones from all projects, not just the first.

James Westby (james-w) wrote :

Found the auto-reset:

   if milestone:
        # get milestone date range
        cur = db.cursor()
        cur.execute('SELECT due_date FROM milestones WHERE name = ?', (milestone,))
        date_end = cur.fetchone()[0]
        cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ?', (date_end,))
        (date_start, in_milestone) = cur.fetchone()

        # if we are already within the milestone, start at the previous
        # milestone's end date; if we are before, start at the beginning of the
        # entire cycle, to allow planning
        if in_milestone:
            ms_sql = "AND w.milestone='%s' AND date >= '%s' AND date <= '%s'" % (
                    escape_sql(milestone), date_start, date_end)
        else:
            ms_sql = "AND w.milestone='%s' AND date <= '%s'" % (
                    escape_sql(milestone), date_end)

I'll have to think about that a bit.

Thanks,

James

Martin Pitt (pitti) wrote :

Right, that's the auto-reset code. BTW, this is not just an issue for resetting the charts, the more important usage is that you need to determine what the "next" milestone is. Ubuntu alpha-2 needs to go until the day Ubuntu alpha-3 starts, not until an intermediate milestone of a different project source.

> > But this will create a combinatorial explosion of charts,
>Because you generate a chart for every milestone? Would it be enough not to generate empty charts?

The svgs are already skipped if they are empty, and empty HTML pages don't cost that much to generate. Of course this won't be a combinatorial explosion, that was a thinko, sorry. It's just additive, so no big deal.

So the main points to consider here are the merging of the milestone table (which is just a small implementation problem), and making milestones "source" specific, so that the individual charts can still know how long a milestone is, and when to reset it.

James Westby (james-w) wrote :

"Martin Pitt" <email address hidden> wrote:

>Right, that's the auto-reset code. BTW, this is not just an issue for
>resetting the charts, the more important usage is that you need to
>determine what the "next" milestone is. Ubuntu alpha-2 needs to go
>until the day Ubuntu alpha-3 starts, not until an intermediate
>milestone of a different project source.

Yep, got that now. I'd never realised what the milestone charts were doing, so thanks for the pointer.

>The svgs are already skipped if they are empty, and empty HTML pages
>don't cost that much to generate. Of course this won't be a
>combinatorial explosion, that was a thinko, sorry. It's just additive,
>so no big deal.

Ok, I'm going to see if I can come up with a cunning plan to reduce the number of these without configuration.

>So the main points to consider here are the merging of the milestone
>table (which is just a small implementation problem),

I've pushed up one approach to this, but I may revisit it depending on the solution to the next problem.

> and making
>milestones "source" specific, so that the individual charts can still
>know how long a milestone is,

I've just talked to Jamie and we want to go a little further than that to give unified burndowns for linaro and ubuntu alphas that are on the same day, but I've no idea if that is feasible. Either way I will aim for no change of behavior for ubuntu teams.

James Westby (james-w) wrote :

> Right, that's the auto-reset code. BTW, this is not just an issue for
> resetting the charts, the more important usage is that you need to determine
> what the "next" milestone is. Ubuntu alpha-2 needs to go until the day Ubuntu
> alpha-3 starts, not until an intermediate milestone of a different project
> source.

I hadn't understood what the milestone charts were doing before. Thanks for the pointer.

> The svgs are already skipped if they are empty, and empty HTML pages don't
> cost that much to generate. Of course this won't be a combinatorial explosion,
> that was a thinko, sorry. It's just additive, so no big deal.

Ok. I'm going to see if I can come up with a cunning way to avoid some of
this without configuration.

> So the main points to consider here are the merging of the milestone table
> (which is just a small implementation problem),

I've pushed up a small change that does this, but may alter the approach depending
on the solution to the next issue.

> and making milestones "source"
> specific, so that the individual charts can still know how long a milestone
> is, and when to reset it.

I've just chatted to Jamie, and this doesn't go quite as far as we would like
in Linaro, where we would like to have consolidated burndown charts for milestones
where the Ubuntu and Linaro milestones are on the same date. I will aim to have
no change for Ubuntu teams when doing this though.

Thanks,

James

247. By James Westby on 2010-12-13

Store the milestone where the project comes from.

This is only a partial solution.

248. By James Westby on 2010-12-14

Merge trunk.

Martin Pitt (pitti) wrote :

Updating status to get a better overview on the "active code reviews" page.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'burndown-chart'
2--- burndown-chart 2010-12-01 15:52:29 +0000
3+++ burndown-chart 2010-12-14 15:53:12 +0000
4@@ -280,6 +280,8 @@
5 if opts.milestone:
6 cur.execute('SELECT due_date FROM milestones WHERE name = ?', (opts.milestone,))
7 else:
8+ # FIXME: this won't work for Ubuntu with Linaro's milestones in
9+ # there.
10 cur.execute('SELECT MAX(due_date) FROM milestones')
11 end_date = cur.fetchone()[0]
12 else:
13
14=== modified file 'collect'
15--- collect 2010-12-10 10:48:07 +0000
16+++ collect 2010-12-14 15:53:12 +0000
17@@ -471,7 +471,7 @@
18 db.cursor().execute('INSERT INTO work_items VALUES (?, ?, ?, ?, ?, date(CURRENT_TIMESTAMP))',
19 (desc, bp_name, status, assignee, milestone))
20
21-def lp_import_milestones(lp_project, db):
22+def lp_import_milestones(milestones, db):
23 '''Import milestones from Launchpad into DB.
24
25 lp_project must be a Launchpad project or distro_series object.
26@@ -483,10 +483,10 @@
27 return
28
29 dbg('lp_import_milestones(): importing from Launchpad')
30- for ms in lp_project.all_milestones:
31+ for ms in milestones:
32 if ms.date_targeted:
33- cur.execute('INSERT INTO milestones VALUES (?, ?)', (ms.name,
34- ms.date_targeted.strftime('%Y-%m-%d')))
35+ cur.execute('INSERT OR IGNORE INTO milestones VALUES (?, ?, ?)', (ms.name,
36+ ms.date_targeted.strftime('%Y-%m-%d'), ms.target.name))
37 db.commit()
38
39 def lp_import_teams(lp, db, cfg):
40@@ -550,25 +550,43 @@
41
42 lp = Launchpad.login_with('ubuntu-work-items', service_root=EDGE_SERVICE_ROOT)
43
44+ urls = []
45+ milestones = []
46+
47+ def add_milestones(project):
48+ milestones.extend([ms for ms in project.all_milestones])
49+
50+ def import_project(project):
51+ add_milestones(project)
52+ url = '%s/%s/+specs?batch=300' % (
53+ report_tools.blueprints_base_url, project.name)
54+ urls.append(url)
55+
56 if 'release' in cfg:
57 lp_project = lp.distributions['ubuntu'].getSeries(name_or_version=cfg['release'])
58+ add_milestones(lp_project)
59 url = '%s/ubuntu/%s/+specs?batch=300' % (
60 report_tools.blueprints_base_url, cfg['release'])
61+ urls.append(url)
62 else:
63 assert 'project' in cfg, 'Configuration needs to specify project or release'
64 lp_project = lp.projects[cfg['project']]
65- url = '%s/%s/+specs?batch=300' % (
66- report_tools.blueprints_base_url, cfg['project'])
67-
68- lp_import_milestones(lp_project, db)
69+ import_project(lp_project)
70+
71+ extra_projects = cfg.get('extra_projects', [])
72+ for extra_project_name in extra_projects:
73+ extra_project = lp.projects[extra_project_name]
74+ import_project(extra_project)
75+
76+ lp_import_milestones(milestones, db)
77 lp_import_teams(lp, db, cfg)
78
79- for (bp, (url, milestone)) in lp_blueprints_from_list(url, name_pattern).iteritems():
80- dbg('lp_import(): downloading %s from %s' % (bp, url))
81- contents = urllib.urlopen(url).read().decode('UTF-8')
82- if lp_import_blueprint(db, bp, url, contents):
83- lp_import_blueprint_workitems(lp, db, bp, url, contents, cfg.get('release'))
84-
85+ for url in urls:
86+ for (bp, (url, milestone)) in lp_blueprints_from_list(url, name_pattern).iteritems():
87+ dbg('lp_import(): downloading %s from %s' % (bp, url))
88+ contents = urllib.urlopen(url).read().decode('UTF-8')
89+ if lp_import_blueprint(db, bp, url, contents):
90+ lp_import_blueprint_workitems(lp, db, bp, url, contents, cfg.get('release'))
91 lp_import_bug_workitems(lp_project, db, cfg)
92
93 ########################################################################
94@@ -725,7 +743,7 @@
95 cur.execute('''CREATE TABLE version (
96 db_layout_ref INT NOT NULL
97 )''')
98- cur.execute('''INSERT INTO version VALUES (6)''')
99+ cur.execute('''INSERT INTO version VALUES (7)''')
100
101 cur.execute('''CREATE TABLE specs (
102 name VARCHAR(255) PRIMARY KEY,
103@@ -759,7 +777,8 @@
104
105 cur.execute('''CREATE TABLE milestones (
106 name VARCHAR(50) PRIMARY KEY,
107- due_date TIMESTAMP NOT NULL
108+ due_date TIMESTAMP NOT NULL,
109+ project VARCHAR(50)
110 )''')
111
112 cur.execute('''CREATE TABLE meta (
113@@ -840,6 +859,13 @@
114 db.commit()
115 ver = 6
116
117+ if ver == 6:
118+ dbg('Upgrading DB to layout version 7')
119+ cur.execute('ALTER TABLE milestones ADD COLUMN project VARCHAR(50)')
120+ cur.execute('UPDATE version SET db_layout_ref = 7')
121+ db.commit()
122+ ver = 7
123+
124 return db
125
126 def create_v5_indexes(cur):
127
128=== modified file 'report_tools.py'
129--- report_tools.py 2010-12-03 12:41:46 +0000
130+++ report_tools.py 2010-12-14 15:53:12 +0000
131@@ -133,9 +133,11 @@
132 if milestone:
133 # get milestone date range
134 cur = db.cursor()
135- cur.execute('SELECT due_date FROM milestones WHERE name = ?', (milestone,))
136- date_end = cur.fetchone()[0]
137- cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ?', (date_end,))
138+ cur.execute('SELECT due_date, project FROM milestones WHERE name = ?', (milestone,))
139+ row = cur.fetchone()
140+ date_end = row[0]
141+ project = row[1]
142+ cur.execute('SELECT MAX(due_date), MAX(due_date) < date(CURRENT_TIMESTAMP) FROM milestones WHERE due_date < ? AND project = ?', (date_end, project))
143 (date_start, in_milestone) = cur.fetchone()
144
145 # if we are already within the milestone, start at the previous

Subscribers

People subscribed via source and target branches

to all changes: