> http://en.wikipedia.org/wiki/IEEE_1541 > > "After a trial period of two years, in 2005 IEEE 1541-2002 was > elevated to a full-use standard by the IEEE Standards Association, and > as of 2007, it was scheduled for maintenance." > > In short: to address the fact that manufacturers, consumers and others > often disagreed on the interpretation of numbers the IEEE and other > organisations decided enough was enough and standardised on a > particular system for unit of measure. > Interesting. I'll be curious to see if KiB (or the other XiB) notations every become commonly used. My guess that KB, MB are fairly firmly entrenched, but it will be interesting to watch the social effect this has. >>> 259 return 0.5892 * dict_size + -0.12295 * ids + \ >>> 260 -0.009595 * total_terms + 23512 >>> >>> It would be nice to see these numbers as constants. On a side note, I >>> know that these numbers worked, but "magic values" always bother me... >>> >> I'm going to leave these as is, b/c this is really a hack that should >> die. I think leaving them in the code encourages someone to think >> very carefully about what they're doing when they change them instead >> of viewing them as constants to be tweaked. In other words, these ARE >> magic values. To me, they're qualitatively different than the >> TIMOUT_SECS (for example) and promoting them to constants at the top >> of the file gives them a status they shouldn't have. It would also >> mean, other code could potentially get at these numbers, something >> else I think we want to avoid without VERY careful consideration. On >> a side note, I continue to be open to suggestions on how to do this >> in Python 2.4 better. IIRC, Dan mentioned that Python 2.6 (I think) >> has a way to determine the size of a structure. > > Can you add an appropriate "deep voodoo" comment above them then? I added this comment: # As noted above, these numbers were estimated through # experimentation. Do not change these unless the # data structure has changed. If it's necessary to change # them, resstimating them experimentally will # be necessary. > >>> Is there a way to tie these numbers to the current version of the >>> index format such that they will become invalidated the next time we >>> change it? (In other words, force us to either review or update these >>> numbers when we change the format.) >>> >> I'm open to suggestions. And, actually, it's not dependent on the >> index format. It's dependent on the structures used to hold that >> information in memory, so tying it to the index format (which to me >> means on disk format) isn't the right thing to do. > > I guess my assumption was that if our index format changed our > structured had, but I realise now that that wouldn't necessarily be true. > >>> >>> 676 except Exception, e: >>> 677 res = False >>> ... >>> 681 if res == 0: >>> 682 res = True >>> 683 return res >>> >>> There's a possible bug here. If an exception happened and res was set >>> to False, line 681 will cause it to be set to True. Python considers 0 >>> and False to be equivalent. >>> >>> However, you could do this instead which is shorter anyway and avoids >>> that possible issue (assuming you didn't intend that): >>> return res and True or False >>> >>> >> Changed to this: >> try: >> try: >> res = \ >> >> ss.consistent_open(self._data_dict.values(), >> self._index_dir) >> except Exception: >> return False >> finally: >> for d in self._data_dict.values(): >> d.close_file_handle() >> if res == 0: >> res = True >> return res > > Okay, I guess. The whole res == 0 still seems odd to me since if res > is False or 0 you will set it to True. Well, really this is a hack to get around the fact that Python considers 0 to be false. Res should never be False. consistent_open returns the version number of the indexes it opened. I've decided to just start versioning with 1 instead of 0 and remove this whole issue. This code now looks like: try: try: res = \ ss.consistent_open(self._data_dict.values(), self._index_dir) except Exception: return False finally: for d in self._data_dict.values(): d.close_file_handle() assert res is not 0 return res
>>> http://cr.opensolaris.org/~bpytlik/ips-search/src/modules/server/repository.py.wdiff.html >>> >>> >>> ========== >>> 185 + output += ("%s %s %s %s\n" % (l[0], >>> l[1], l[2], l[3])) >>> >>> Why not something like: >>> >>> output += "%s\n" % ' '.join(l) ? >>> >>> Is it safe to print all of the elements of l? >>> >> I think it's clearer this way. It shows that exactly 4 things are >> being printed (which is what the client side code is expecting). >> Also, it makes clear that the things in l are ordered and that the >> order is important for the client. > > Actually that isn't clear to me. Can you add a comment? I've added: # The query_engine returns four pieces of information in the # proper order. Put those four pieces of information into a # string that the client can understand. >>> 188 + self.cfg.catalog.refresh_index() >>> >>> I'm somewhat concerned about the effect on a distro import using >>> solaris.py or any other bulk pkgsend operations that this is going to >>> have. >>> >>> I think we would need the ability to defer any index rebuild until a >>> depot restart or something similar. >>> >> I'm open to suggestions on where to put this, but I don't understand >> the issue really. Is your concern that this will slow down a distro >> import, or that it will break a distro import? Why would you want to >> defer an index rebuild when you can asynchronously update the index >> while waiting for more data to come through the pipe? > > My concern is the slow down and the additional function call overhead. > The desire to rebuild the index is good. The place where it's being > called is what I have a problem with. I know we've talked about this some more. Are you comfortable with the idea that attempting to rebuild the index after each package is a good idea since they're batched together and run when available? >> Thanks again for taking a look Shawn. >> I'll be posting an updated webrev in a little bit once I verify I >> haven't broken anything and synced up with the gate. > > Remember to post it as a *separate* webrev so you don't overwrite the > old one. > > Don't be offended by my style nitpicks. Most of them are not my > personal preference, they're just comments based on the existing code > and being consistent with it. > > I've worked in projects that have one-space indentation to 8-space tab > indentation. It doesn't really matter to me. Okay, I draw the line at > GNU-style braces :-) > > Thanks, I'm not offended :) But, I'm also tend to have strong opinions on some style things, like the parenthesis issue. Apparently we've decided to follow Danek's lead, so you and he will probably have to continue to point them out to me because it will take a while for me to switch so that what I think is good style is bad and the other way around. I'm going to fix whatever I've broken, then post another (versioned) webrev. Brock _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
