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

[Barros Pena, Belen]

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