+1 to everything daviddavis just said. Dana Walker
Associate Software Engineer Red Hat <https://www.redhat.com> <https://red.ht/sig> On Tue, Jun 12, 2018 at 5:11 PM, David Davis <davidda...@redhat.com> wrote: > On Tue, Jun 12, 2018 at 4:50 PM, Brian Bouterse <bbout...@redhat.com> > wrote: > >> Silly question, but could we just call our 'id' 'pk' instead? Since that >> is a fully reserved value in Django for the primary key it seems clearest >> to just use that? What about that? >> > > Are you recommending we rename the id field to pk in the database? I’m not > sure if that would work. > > >> >> On Tue, Jun 12, 2018 at 3:44 PM, Jeff Ortel <jor...@redhat.com> wrote: >> >>> On 06/08/2018 02:57 PM, Brian Bouterse wrote: >>> >>>> >>>> @jortel: We're blocked on your -1 vote expressed for 3704. We have >>>> practical plugin writer issues with the current state. Can you elaborate on >>>> why we shouldn't go forward with https://pulp.plan.io/issues/3704 >>>> >>> >>> The 'ID' column is reserved for the primary key and is inappropriate for >>> natural keys. This is well establish convention and best practice. >> >> >> I don't understand this reasoning. Earlier in the thread we discussed how >> the sources recommending these conventions also mention that if we have a >> practical reason or problem with that convention to do something >> differently. We received complaints on this name about collisions so I >> don't follow how we should still follow the convention. >> >> >>> Plugin writers specify natural keys. Also, by introducing '_' prefix >>> (or any prefix) means a table could have both 'ID' and '_ID' columns which >>> is especially confusing since the 'ID' column would not be the primary key. >>> >> >> We have two concepts here that are similar, so I think that problem is >> mostly unrelated to this decision. For example, if we leave the names as-is >> we have this problem only now it's named id and errata_id and in addition >> we'll have the problems listed below. >> >> >>> How does naming the natural key for an rpm as 'rpm_id' cause a >>> significant problem for plugin writers? >>> >> >> It's a good question because it's the whole motivation for this change. >> It's not an rpm, it's an erratum which doesn't have nevra like a package. >> It's also the problem from another content type I heard about at Config >> Management Camp. >> >> It causes problems in two ways: >> >> * plugin users (not writers) who are familiar with 'id' as part of the >> erratum data type would then have to also understand this field name >> renaming that Pulp arbitrarily introduces. This could get confusing when >> the user submit a filter with id='ID-2115858' and they find nothing because >> 'id' is matching on the primary key not on the 'id' attribute of the errata >> like they expect. Those users would also be Pulp users so they'll >> understand that _id means the pk. >> > > By the same logic, if Pulp users know that id means pk, wouldn’t they > therefore understand that the id is not the erratum id? > > >> >> * plugins specifically may wrap other tools and now they have to maintain >> mappings as well. This is specifically the case with errata which the data >> model is design to be name-for-name identical to the createrepo_c interface >> >> > Mapping one field to another seems rather minor. Or am I missing something? > > >> >>> @bmbouters: just curious, where does the rpm 'id' come from and how is >>> it used differently than the NEVREA composite natural key. >>> >>> >>> _______________________________________________ >>> Pulp-dev mailing list >>> Pulp-dev@redhat.com >>> https://www.redhat.com/mailman/listinfo/pulp-dev >>> >> >> >> _______________________________________________ >> Pulp-dev mailing list >> Pulp-dev@redhat.com >> https://www.redhat.com/mailman/listinfo/pulp-dev >> >> > > _______________________________________________ > Pulp-dev mailing list > Pulp-dev@redhat.com > https://www.redhat.com/mailman/listinfo/pulp-dev > >
_______________________________________________ Pulp-dev mailing list Pulp-dev@redhat.com https://www.redhat.com/mailman/listinfo/pulp-dev
