launchpad-reviewers team mailing list archive

[Michael Nelson <michael.nelson@xxxxxxxxxxxxx>]

On Fri, Aug 27, 2010 at 12:03 PM, Stuart Bishop
<stuart.bishop@xxxxxxxxxxxxx> wrote:
> Review: Approve db
> Confused myself. This is all fine. The difference references the derived distro series it is for. The derived distro series references its parent distro series.
>
> Add the index and you are good to go from my pov.

Thanks Stuart. Based on our conversation [1] I've updated to remove
the activity_log column and instead added a joining table for messages
for the DistroSeriesDifference (r9690, incremental attached). I've
based it on the similar BugMessage table, but please check it in case
I've missed something.

[1] 12:04 < noodles775> stub: Why would we want delete permissions? At
the moment I'm planning that these differences will end in a resolved
state, but we wouldn't want to delete them until the derived
distroseries is itself deleted (if it ever is).
12:05 < stub> The derived distribution has a patched version of
firefox. Tomorrow it decides that it should just use the parents
version of firefox.
12:07 < noodles775> Great, so assuming the parents version has a
higher revno, they upload it, this record is marked as resolved and
the activity log is updated, but the record is not deleted, as
tomorrow the parent might upload a new version, and the previous
comments/activity is still useful.
12:07 < stub> ok
12:08 < stub> How big do you think the comments and activity will grow?
12:08 < stub> It will be a pain to split out into a structured format later.
12:10 < stub> If it is append only, storing it as a single text blob
isn't ideal. If it is more a whiteboard or a copy of a text file
stored in a branch, that would be fine I guess.
12:10 < noodles775> On average 4 or 5 lines. If a particular
difference goes through resolved->needs attention-> resolved it would
be a few more.
12:10 < stub> But over time
12:10 < noodles775> Yes, it is just like a whiteboard.
12:11 < stub> ok
12:11 < noodles775> Yep, so if overtime it went through that cycle
(resolved->needs attention->resolved) *lots*, there would be 2 lines
for each change.
12:11 < noodles775> Do you think its worth adding a separate comment model?
12:12 < noodles775> (from memory the code/bugs guys made the comment
model re-usable... I'll check).
12:14 < noodles775> stub: oh, and I think I misunderstood your comment
above. I am making the activity_log append only.
12:17 < stub> So we would probably be better off with a separate table
containing the activity, one row per addition. Or even a link table
between distroseriesdifference and message same as our other comment
spools.
12:18 < noodles775> Yep, just looking at the message table... I'll do that then.
12:18 < noodles775> Thanks stub.
12:19 < stub> That gives you the infrastructure for attachments, email
interfaces etc. if we want them in the future.
12:20 < noodles775> Yeah, given that the table already exists, it
sounds crazy not to use it.

-- 
https://code.launchpad.net/~michael.nelson/launchpad/distro-series-difference-schema/+merge/33515
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~michael.nelson/launchpad/distro-series-difference-schema into lp:launchpad.

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-08-27 06:50:09 +0000
+++ database/schema/comments.sql	2010-08-27 10:45:34 +0000
@@ -508,10 +508,14 @@
 COMMENT ON COLUMN DistroSeriesDifference.derived_series IS 'The derived distroseries with the difference from its parent.';
 COMMENT ON COLUMN DistroSeriesDifference.source_package_name IS 'The name of the source package which is different in the two series.';
 COMMENT ON COLUMN DistroSeriesDifference.last_package_diff IS 'The most recent package diff that was created for this difference.';
-COMMENT ON COLUMN DistroSeriesDifference.activity_log IS 'A log of actions and/or comments by users regarding action to be taken.';
 COMMENT ON COLUMN DistroSeriesDifference.status IS 'A distroseries difference can be needing attention, ignored or resolved.';
 COMMENT ON COLUMN DistroSeriesDifference.difference_type IS 'The type of difference that this record represents - a package unique to the derived series, or missing, or in both.';
 
+-- DistroSeriesDifferenceMessage
+COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.';
+COMMENT ON COLUMN DistroSeriesDifferenceMessage.distro_series_difference IS 'The distro series difference for this comment.';
+COMMENT ON COLUMN DistroSeriesDifferenceMessage.message IS 'The comment for the distro series difference.';
+
 -- DistroSeriesPackageCache
 
 COMMENT ON TABLE DistroSeriesPackageCache IS 'A cache of the text associated with binary packages in the distroseries. This table allows for fast queries to find a binary packagename that matches a given text.';

=== renamed file 'database/schema/patch-2208-99-0.sql' => 'database/schema/patch-2208-07-0.sql'
--- database/schema/patch-2208-99-0.sql	2010-08-24 14:55:03 +0000
+++ database/schema/patch-2208-07-0.sql	2010-08-27 10:52:17 +0000
@@ -5,7 +5,6 @@
     derived_series integer NOT NULL CONSTRAINT distroseriesdifference__derived_series__fk REFERENCES distroseries,
     source_package_name integer NOT NULL CONSTRAINT distroseriesdifference__source_package_name__fk REFERENCES sourcepackagename,
     last_package_diff integer CONSTRAINT distroseriesdifference__last_package_diff__fk REFERENCES packagediff,
-    activity_log text,
     status integer NOT NULL,
     difference_type integer NOT NULL
 );
@@ -13,5 +12,15 @@
 CREATE INDEX distroseriesdifference__source_package_name__idx ON distroseriesdifference(source_package_name);
 CREATE INDEX distroseriesdifference__status__idx ON distroseriesdifference(status);
 CREATE INDEX distroseriesdifference__difference_type__idx ON distroseriesdifference(difference_type);
-
-INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0);
+CREATE INDEX distroseriesdifference__last_package_diff__idx ON distroseriesdifference(last_package_diff);
+
+CREATE TABLE DistroSeriesDifferenceMessage(
+    id serial PRIMARY KEY,
+    distro_series_difference integer NOT NULL CONSTRAINT distroseriesdifferencemessage__distro_series_difference__fk REFERENCES distroseriesdifference,
+    message integer NOT NULL CONSTRAINT distroseriesdifferencemessage__message__fk REFERENCES message,
+    UNIQUE (distro_series_difference, message)
+);
+CREATE INDEX distroseriesdifferencemessage__distroseriesdifference__idx ON distroseriesdifferencemessage(distro_series_difference);
+CREATE INDEX distroseriesdifferencemessage__message__idx ON distroseriesdifferencemessage(message);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 07, 0);

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-08-27 06:50:09 +0000
+++ database/schema/security.cfg	2010-08-27 10:45:11 +0000
@@ -135,6 +135,7 @@
 public.distributionsourcepackage        = SELECT, INSERT, UPDATE, DELETE
 public.distributionsourcepackagecache   = SELECT
 public.distroseriesdifference           = SELECT, INSERT, UPDATE
+public.distroseriesdifferencemessage    = SELECT, INSERT, UPDATE
 public.distroserieslanguage             = SELECT, INSERT, UPDATE
 public.distroseriespackagecache         = SELECT
 public.emailaddress                     = SELECT, INSERT, UPDATE, DELETE