[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [libosinfo 5/8] Add osinfo_db_identify_media



On Mon, Dec 10, 2012 at 10:45:23PM +0200, Zeeshan Ali (Khattak) wrote:
> On Mon, Dec 10, 2012 at 11:20 AM, Christophe Fergeau
> <cfergeau redhat com> wrote:
> > On Wed, Dec 05, 2012 at 06:42:43PM +0200, Zeeshan Ali (Khattak) wrote:
> >> > +
> >> > +/**
> >> > + * osinfo_db_identify_media:
> >> > + * @db: a #OsinfoDb database
> >> > + * @media: the installation media
> >> > + * data
> >> > + *
> >> > + * Try to match the @media created using osinfo_media_create_from_location()
> >>
> >> This makes it sound like app developer doesn't have a choice. As an
> >> app developer, I'd think why is libosinfo not creating the media
> >> instance for me if it knows that I'll be doing that just before this
> >> call anyways.
> >>
> >> I recall that you are doing it this way because implementing async
> >> version of this method will than be very difficult?
> >
> > Not really difficult, but we already have async methods that can be used to
> > read the needed info from an image
> > (osinfo_media_create_from_location{_async}). It also does not strike me as
> > so bad to separate actual disk IO from DB lookups,
> 
> Only that there is no advantage to app developers, only minor inconvenience.

Well, adding more async functions  also means a slightly bigger API to
figure out, a bit more changes to go from the old API to the new one, ...
These are also minor inconvenience for developers.

> 
> > Adding an all-in-one
> > method would mean at least 3 more API calls, redundant with the existing
> > functions, ...
> > so I prefer to stay with the standalone _identify_media
> > call. Nothing prevents us from adding more calls later if they are really
> > needed, removing redundant calls is harder.
> 
> IMO we already know that they are needed.

"if they are *really* needed". As we can't remove functions once they get
into a release, I'd rather that we live without these functions for now,
and see how different apps use the OsinfoMedia class, and if this
additional API would make their life much better, then we can add it
knowing it's not API we'll have to deprecate 3 months later.

Christophe

Attachment: pgpxVT3254aK7.pgp
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]