[Toaster] [review-request][PATCH] toaster: make 0021 migration compatible with MySQL

Mon Aug 24 03:14:39 PDT 2015

On 17 August 2015 at 19:25, Brian Avery <avery.brian at gmail.com> wrote:

> I'm fine with the polite refusal approach.  It has less magic
> associated with it.
>

There is one issue with polite refusal: if a user doesn't want/need to use
bitbake to manually trigger builds, the default project isn't necessary.
The default project is only required to collect builds which are not
triggered from the Toaster UI. So I'm not sure that we want to enforce
setting a default project, as it's not relevant to every user.

> I'm also a tad confused on the user workflow for this.
>

At the moment, if builds are coming out of bitbake which were not triggered
by Toaster, those builds need a project to be associated with, so they can
be shown in the UI. The default project is there for that reason (as far as
I can tell).

Builds triggered by Toaster are associated with a project, of course.

> If we had to have an autocreated defaultproject I'd still prefer a
> flag in that it allows the interface (at a future time) to change the
> default project. Imagine a case where you have been running for awhile
> and collecting builds into default project id=0. Now you want to
> collect all builds under the project id=3 for the weekend because you
> are debugging a particular issue. Then they might very well switch it
> back to project id=0 on monday.
>

My main issue with using a project with ID 0 is that MySQL won't allow it,
which is what causes the bug when starting Toaster (what I was originally
trying to fix).

Putting that to one side, is_default (or similar) seems less "magic" than
using a project with ID 0, as it explicitly marks the record which is the
default project. At the moment, the default project is implicitly marked by
its ID.

I would look to enforce the 1 default project restriction at the logic
> level rather than the dbase level.
>

Agreed.

So that I can actually resolve this (it is a blocker for me), can we agree
on an implementation? Summarising the above, I suggest these steps:

1. The user can specify the default project (the project for manual bitbake
builds) when they start Toaster (I need to check whether this is already
implemented). This sets a field in the Project table which marks the
project for collecting bitbake builds initiated outside Toaster. If the
project ID doesn't exist, we don't start Toaster. If no default project is
specified, we start Toaster without one (see below).

NB Belen asked whether this should be a project-specific parameter, or
should be somehow appended to the build. As it's Toaster-specific, I'm not
sure we can modify bitbake to support this (note that it is only relevant
to builds triggered outside Toaster).

2. If the user doesn't specify a default project, the one already marked in
the db (if it exists) is used.

3. If there is no default project yet, a project is created and the
"is_default" field is set. Though I'm wondering whether migrations should
create the default project rather than runtime code. (At the moment, if a
build is added to the db which doesn't have a project specified, the
default project is created at the same time as the build is added. But I
don't like this kind of side effect.) Any preferences?

4. I modify the migrations scripts so we don't set the project ID for a
build to 0 by default.

5. When a build is added without a project, the default project is looked
up and used as the project for that build.

6. A remaining issue is compatibility with existing installations. There
may be users who already have a history of builds associated with project
ID 0. I think I could add a migration which takes an existing project with
ID 0 in the db and reconstructs it with a different (auto-generated) ID,
and with the is_default flag set to true. I'd also need to update any other
tables which reference the old ID.

Does this seem sensible?

Another alternative would be to allow builds which aren't associated with a
project, though this would have lots of repercussions. I'm not sure we want
to go there right now.

Elliot

>
> -b
>
> On Mon, Aug 17, 2015 at 11:01 AM, Barros Pena, Belen
> <belen.barros.pena at intel.com> wrote:
> >
> >
> > On 17/08/2015 10:52, "toaster-bounces at yoctoproject.org on behalf of
> > Damian, Alexandru" <toaster-bounces at yoctoproject.org on behalf of
> > alexandru.damian at intel.com> wrote:
> >
> >>So the whole point of the scheme is:
> >>
> >>
> >>- when starting builds in command line mode, to which project the builds
> >>should belong ?
> >>
> >>
> >>The answer is: the user must set the project by specifying a parameter,
> >>something like:
> >>
> >>"source toaster start project=3"
> >
> > Does this mean that I cannot change to which project the builds go
> without
> > stopping and restarting Toaster? Just asking: I am still trying to
> > understand how this workflow works / should work, to be honest. My first
> > thought was that the project should be a build parameter, not a
> > Toaster-wide parameter, but I am not 100% sure.
> >
> > Also, is this currently working, i.e can I currently pass a project id
> > when I source the Toaster script?
> >
> >>
> >>
> >>- when the user doesn't set this parameter, what happens ?
> >
> > Well, maybe Toaster should politely refuse to start until you tell it in
> > which project you want to store the build information. That would remove
> > the need for a default project.
> >
> > Cheers
> >
> > Belén
> >
> >
> >>
> >>
> >>My answer was: assign them to default project 0. It's a magic id, but
> >>it's not any less magic than having a project flag "is_default_project",
> >>and enforcing it to be 1 for a single entry (I'm
> >> not sure even how to write this restriction in SQL).
> >>
> >>
> >>I chose the ID 0 as the magic ID because we wanted backwards
> >>compatibility with existing installations. People already had projects in
> >>the database, starting with ID 1, so it was not clear what
> >> ID will be available to set as the default ID - it would be instance and
> >>time dependent, depending on when people applied the upgrade. Since the
> >>IDs started with 1, on all installations, I knew that ID 0 was available,
> >>and voila.
> >>
> >>
> >>The field definition with (default=0) is a mistake in the original
> >>commit. It is right to remove it, and not replace it with anything else
> >>in terms of the field definition.
> >>
> >>
> >>
> >>Cheers,
> >>
> >>Alex
> >>
> >>
> >>
> >>
> >>
> >>
> >>On Fri, Aug 14, 2015 at 7:49 PM, Brian Avery
> >><avery.brian at gmail.com> wrote:
> >>
> >>Having a flag rather than relying on a magic id number does seem
> >>preferable.
> >>
> >>-brian
> >>
> >>On Fri, Aug 14, 2015 at 6:40 AM, Smith, Elliot <elliot.smith at intel.com>
> >>wrote:
> >>> On 14 August 2015 at 14:19, Smith, Elliot <elliot.smith at intel.com>
> >>>wrote:
> >>>>
> >>>> On 14 August 2015 at 13:26, Smith, Elliot <elliot.smith at intel.com>
> >>>>wrote:
> >>>>>
> >>>>> On 14 August 2015 at 11:51, Damian, Alexandru
> >>>>> <alexandru.damian at intel.com> wrote:
> >>>>>>
> >>>>>> One of intended effects of this migration is to make sure that we
> >>>>>>have a
> >>>>>> Project with id "0" and release None after this migration is run.
> >>>>>>
> >>>>>> After this patch is merged, there is no guarantee that this entry in
> >>>>>>the
> >>>>>> database will exist, unless I'm missing something.
> >>>>>>
> >>>>>> Can you please add code to the migration to automatically add the
> >>>>>> Project id 0 entry if it does not exist ?
> >>>>>
> >>>>>
> >>>>> Is this to ensure that there is a "Default project" in the database
> >>>>>with
> >>>>> ID 0? From what I've seen, the default project is the one which
> >>>>>builds are
> >>>>> attached to if they are started outside of Toaster by bitbake. So is
> >>>>>the
> >>>>> intention to ensure that any Build object will get a project ID of 0
> >>>>>if that
> >>>>> object is not explicitly associated with a Project when it is
> created?
> >>>>
> >>>>
> >>>> I did some more digging and found that the issue is with the ID being
> >>>>0,
> >>>> rather than 1, as MySQL autoincrement fields start at 1
> >>>>
> >>>>(
> http://stackoverflow.com/questions/20328905/south-migration-database-ba
> >>>>ckend-does-not-accept-0-as-a-value-for-autofield).
> >>>>
> >>>> If I change to default=1 for the project_id field, there's no problem,
> >>>>and
> >>>> I just need no_dry_run = True to get migrations working with MySQL.
> >>>>Would
> >>>> this be an acceptable solution? (I'm assuming there's nowhere in the
> >>>>code
> >>>> base where a project ID of 0 is hard-coded.)
> >>>
> >>>
> >>> I'm not sure my suggestion will work as I intended. I think this could
> >>>cause
> >>> problems because the ORMWrapper in buildinfohelper.py does a
> >>>get_or_create()
> >>> for a Project with ID=0, name="Default Project". If this is changed to
> >>>ID=1,
> >>> name="Default Project", this could cause a clash if there is an
> existing
> >>> project with ID=1 (e.g. the db can't find a default project with ID 1,
> >>>so it
> >>> tries to create one, but the ID field clashes with an existing project
> >>>and
> >>> it fails).
> >>>
> >>> However, if the get_or_create() is changed to just name="Default
> >>>Project",
> >>> the correct project should be returned (though again, this could be
> >>> problematic if the user has a project called "Default Project").
> >>>
> >>> I think the existing code works OK because project ID is an
> >>>autoincrement
> >>> field, so any projects created by the user will have an ID of 1+.
> >>>Creating
> >>> one with an ID of 0 as the default won't cause a clash. But this
> doesn't
> >>> work with MySQL, as an autoincrement field can't have a value of 0.
> >>>
> >>> I'd suggest instead that we mark one of the Project records as the
> >>>default
> >>> using a flag ("is_default_project"), rather than relying on its ID or
> >>>name.
> >>> This would then be the project created by ORMWrapper if there is no
> >>>existing
> >>> default project.
> >>>
> >>> We could always retrieve this project correctly by looking for a single
> >>> Project with is_default_project = True. There is still a possibility of
> >>>a
> >>> problem (e.g. if more than one Project has this field set to True), but
> >>>I
> >>> think this is a better solution than relying on ID or name.
> >>>
> >>> Elliot
> >>> --
> >>> Elliot Smith
> >>> Software Engineer
> >>> Intel Open Source Technology Centre
> >>>
> >>
> >>
> >>> --
> >>> _______________________________________________
> >>> toaster mailing list
> >>> toaster at yoctoproject.org
> >>>
> >>https://lists.yoctoproject.org/listinfo/toaster
> >><https://lists.yoctoproject.org/listinfo/toaster>
> >>>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>--
> >>Alex Damian
> >>Yocto Project
> >>
> >>SSG / OTC
> >>
> >>
> >>
> >
> --
> _______________________________________________
> toaster mailing list
> toaster at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>

-- 
Elliot Smith
Software Engineer
Intel Open Source Technology Centre
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150824/39683aec/attachment-0001.html>