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

Mon Aug 24 03:39:28 PDT 2015

On 24/08/2015 11:14, "Smith, Elliot" <elliot.smith at intel.com> wrote:

>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.

Right, I haven't been very clear. This is only required when you start
Toaster by sourcing the toaster script

"source toaster start"

When you start Toaster like so, you are explicitly telling us that you
will not be starting your builds from Toaster, but in some other way, and
the need to specify a project applies always. Only then we require you to
specify the project to which your builds should go:

"source toaster start project=3"

If you start Toaster the build mode way:

bitbake/bin/toaster

We don't require you to specify the project, because you are explicitly
telling us that you will be starting your builds from Toaster and you will
specify the project via the Toaster interface.

> 
>
>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).

I am starting to sound like an scratched vinyl :/ Projects are not just a
configuration and a bucket for builds. From the users's perspective, they
are a tool to organise their work. Having a dumpster project, like our
current 'default project', doesn't help users when you consider projects
from this perspective. It might make it harder for them to find their
builds data. 

>
>
>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).

Well, sorry, but from a design perspective this default project thing does
not sound like a very good idea.

>
>
>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).

What this means for people using Toaster in 'interactive' mode is that
they will have to stop and restart Toaster every time they want to store
their builds data in a different project. Once again, from a design
perspective, this is pretty dreadful.

>
>
>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.

See above: this default project thing, not great.

>
>
>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.

No, this is not good either. I am starting to think we have rushed into
this without thinking the stuff through. It might be worth taking some
time to consider this carefully. My take on things from the beginning has
been: if it's not going to be good enough, it's not worth doing. We seem
to have gotten ourselves in one of those kind of situations.

Cheers

Belén

>
>
>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
>
>
>
>