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

[Brian Avery]

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

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

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.

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

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