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

[Brian Avery]

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