Not sure I agree. I like Shawn's proposal. Why do API consumers need to know what's going on under the covers? Isn't that the point of the API to begin with?
-j On Fri, Oct 24, 2008 at 02:23:54PM -0700, Brock Pytlik wrote: > John Rice wrote: > > Great that looks simpler again - we'll make the changes when are back in > > on Tuesday, test to make sure this does what's needed for both the GUI > > clients and respin. If Danek hasn't found a fix for the __init__.py > > issues we'll revert this file to r590. > > > > Bug logged to track root cause analysis of problem post 2008.11 (4116 > > depends on this): > > 4231:Resolve root cause for memory leak and remove workaround for 4116 > > > > Brock you happy with this approach, as opposed to modifying the API? > > > Actually, I find modifying the api cleaner than this. This just makes my > skin crawl personally. I'm not inherently opposed to leaving the API > workaround in for a while we get things fixed up. I think an API option > of "prefer low memory use over blazing speed" is at least a possibly > reasonable switch to provide. Hard coding in our pkg client name here > seems wrong to me on many levels. If this is what everyone else likes, > I'm fine with it for 2008.11, but this workaround I want out of the code > as soon as possible. > > Brock > > JR > > > > Shawn Walker wrote: > > > >> jmr wrote: > >> > >>> Michal has respun his webrev without the actions:__init__.py revert > >>> to r590 as well, to give us both options. > >>> > >>> With __init__.py from the trunk: > >>> http://cr.opensolaris.org/~migi/23_10_2008_bug_4116_v1_no_init/ > >>> > >>> With __init__.py from rev590: > >>> http://cr.opensolaris.org/~migi/23_10_2008_bug_4116_v1/ > >>> > >> Earlier today, it occurred to me that we know which pkg client is > >> currently using the API via history. As such, rather than add a > >> temporary flag to the api to control cache usage, unintentionally > >> exposing it, it makes more sense to just limit the caching to the cli > >> as a temporary workaround instead. > >> > >> To do this, I would remove all of the use_cache changes you've made > >> and make the following simple changes to modules/client/image.py: > >> > >> diff -r c95448e970e5 src/modules/client/image.py > >> --- a/src/modules/client/image.py Fri Oct 24 15:36:26 2008 -0500 > >> +++ b/src/modules/client/image.py Fri Oct 24 15:42:28 2008 -0500 > >> @@ -935,11 +935,14 @@ > >> """Find on-disk manifest and create in-memory Manifest > >> object, applying appropriate filters as needed.""" > >> > >> - if fmri in self.__manifest_cache: > >> - m = self.__manifest_cache[fmri] > >> + if self.history.client_name == "pkg": > >> + if fmri in self.__manifest_cache: > >> + m = self.__manifest_cache[fmri] > >> + else: > >> + m = self.__get_manifest(fmri) > >> + self.__manifest_cache[fmri] = m > >> else: > >> m = self.__get_manifest(fmri) > >> - self.__manifest_cache[fmri] = m > >> > >> self.__touch_manifest(fmri) > >> > >> > >> ========================== > >> > >> The above change is only intended to be temporary, and a bug would > >> need to be filed to address the real issue. However, this to me, is > >> far simpler and preferable to adding a use_cache flag which also bumps > >> the API version and so on. > >> > >> Code Review follows: > >> src/packagemanager.py: > >> line 40: s/lenght/length/ > >> > >> lines 580, 992-994: Suggested word change s/Fetching/Retrieving/ > >> > >> line 1112: tue users? what's that? > >> > >> Otherwise, looks fine. > >> > >> Cheers, > >> > > > > _______________________________________________ > > pkg-discuss mailing list > > [email protected] > > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > > > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
