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 > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
