On Fri, Sep 11, 2009 at 01:02:49PM -0500, Tom Mueller wrote: > Mark, > Thanks for making those changes. > > Can you close the file descriptor for the temporary file? > > Thanks. > Tom
Still not an unreasonable request, though done implicitly through garbage collection. Anyway: http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.3-4/ --Mark > Mark J. Nelson wrote: >> Newly updated webrevs: >> >> - http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.3/ >> - http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.1-3/ >> - http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.2-3/ >> >> Shawn: Using the depotcontroller refresh() method is causing a race >> condition. It's triggering an index refresh, but is returning after >> sending the SIGUSR1, so sometimes the subsequent search is succeeding >> and sometimes it's failing. If I sleep after the dc.refresh(), it >> always succeeds (inappropriately). So I removed the refresh(). >> >> On Fri, Sep 11, 2009 at 09:30:14AM -0500, Tom Mueller wrote: >> >>> Mark, >>> >>> t_pkgsend.py, line 377: >>> Can you please use the urlparse module for constructing the >>> file-based URL? Generally, concatenating strings for creating URLs >>> is not portable across platforms. The urlparse module will also do >>> the right encoding for spaces in filenames, etc. >>> >> >> I used urllib.pathname2url(); see webrevs. I didn't fix existing >> usage. >> >> >>> I see that this file also uses the same form of URL construction in >>> other tests, so if you want to consider this a different bug, that >>> would be fine too. >>> >> >> The existing usage could be a different bug; I don't know how strongly >> people feel about this. >> >> >>> Using /bin/ls as a file to transmit will also be problematic on >>> Windows. We aren't currently able to run the CLI tests on Windows, >>> so if you want to consider that a separate bug, that is fine too. >>> >> >> No, that's a perfectly reasonable request. I generated a tempfile >> instead. >> >> --Mark >> >> >>> Thanks. >>> Tom >>> >>> Mark J. Nelson wrote: >>> >>>> Ok, I think I have addressed all concerns (and that Danek's questions >>>> were mosty answered by Shawn.) >>>> >>>> Here are the new and incremental webrevs: >>>> >>>> http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.2/ >>>> http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control.1-2/ >>>> >>>> Some notes on testing: I did almost exactly as Shawn suggested, with >>>> two primary differences: >>>> >>>> 1. I have independently verified that index suppression is working >>>> completely as intended for the following two cases: >>>> >>>> A. No depot server running, using pkgsend only with a file uri >>>> B. Depot server running, using pkgsend only with a http uri >>>> >>>> I have further verified that, when you are running a depot server >>>> but publishing to the same repo via a file uri, the open and add >>>> pkgsend subcommands trigger a silent index refresh. So whatever >>>> was previously published, but not indexed, will magically be >>>> indexed, even though the currently open transaction will not be. >>>> >>>> This led me on a merry chase to figure out why my test searches >>>> were returning results unexpectedly, and eventually led to a slight >>>> reordering of test operations. (Checking for search failure on the >>>> http-published package prior to publishing via file transaction.) >>>> >>>> 2. If you publish an empty package, a "pkg search pkg:::" will always >>>> fail, even after refreshing the search indices. >>>> >>>> So I added a file action to each of my test packages. >>>> >>>> I'm inclined to treat difference (1) as "not part of this wad." Is >>>> this expected behavior? If not, I can collect info and file a bug. >>>> >>>> Since the tests were not previously reviewed, I welcome comments on >>>> them. >>>> >>>> --Mark >>>> >>>> >>>> >>>> On Thu, Sep 10, 2009 at 04:59:55PM -0500, Shawn Walker wrote: >>>> >>>>> Danek Duvall wrote: >>>>> >>>>>> Mark J. Nelson wrote: >>>>>> >>>>>> >>>>>>> http://cr.opensolaris.org/~mjnelson/webrev.pkg-index-control/ >>>>>>> >>>>>> depot.py: >>>>>> >>>>>> - line 729: I forget -- is there a reason we can't send back a 5xx >>>>>> error >>>>>> code? (This may be more of a question for Shawn.) Alternatively, >>>>>> why >>>>>> can't both error returns be NOT_FOUND? See file_0(). >>>>>> >>>>> We currently don't send back 5xx error codes because that causes >>>>> Apache to think that the backend has stopped working and it will >>>>> refuse requests. >>>>> >>>>> We don't send 404 for most of the failed operations because >>>>> versioned_urlopen will interpret that to mean that the server >>>>> doesn't support this operation as opposed to the operation >>>>> failing. This comes back to us not having upgraded to the >>>>> newest version of cherrypy yet, which would allow us to have >>>>> custom status messages (i.e. 404 No write permissions on search >>>>> indices.). >>>>> >>>>> Cheers, >>>>> -- >>>>> Shawn Walker >>>>> >>>> _______________________________________________ >>>> pkg-discuss mailing list >>>> [email protected] >>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >>>> >> >> >>> begin:vcard >>> fn:Tom Mueller >>> n:Mueller;Tom >>> org:Sun Microsystems, Inc.;Update Center Software >>> adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA >>> email;internet:[email protected] >>> title:Senior Staff Engineer >>> tel;work:877-250-4011 >>> tel;fax:877-250-4011 >>> tel;home:402-916-9943 >>> x-mozilla-html:TRUE >>> version:2.1 >>> end:vcard >>> >>> >> >> > > begin:vcard > fn:Tom Mueller > n:Mueller;Tom > org:Sun Microsystems, Inc.;Update Center Software > adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA > email;internet:[email protected] > title:Senior Staff Engineer > tel;work:877-250-4011 > tel;fax:877-250-4011 > tel;home:402-916-9943 > x-mozilla-html:TRUE > version:2.1 > end:vcard > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
