On Mon, May 18, 2009 at 06:14:33PM -0500, Shawn Walker wrote: > Danek Duvall wrote: >> On Fri, May 15, 2009 at 05:27:54PM -0500, Shawn Walker wrote: >> >>> http://cr.opensolaris.org/~swalker/pkg-8724/ > >> - line 375: legal values for collection_type? Or does anything care? > > What is this referencing? > > pkg.server.api?
Yup, sorry. >> depot.py: >> - line 809: could this list be kept in publisher.Repository? Or maybe >> in the repositoryconfig code? > > Not certain; this is a specific list of attributes as opposed to all of > them. If I were to add a new attribute, how would I know to add it here as well? Or whether to add it at all? >> - line 840: another case where we need to return 404 instead of 500 for >> some reason, but can't yet? > > No, because returning a 500 causes Apache to flip out and stop forwarding > requests to the backend. Also, the HTTP specification says that a server > can return a 404 when it doesn't want to know why a resource is > unavailable. Okay. I just hate that we're losing information here, but since the error gets logged and sent to the client (though it probably won't be seen there) not much gets lost. >> - line 850: I'm not sure I see the point of being able to name just a >> single package. > > It is still useful. A claim you make without any substantiation. Though I don't now remember what it was I was missing. <sigh> Stupid known unknown unknowns. >> p5i.py: >> >> - shouldn't client/api.py be modified to use these routines? > > I didn't want to make any unnecessary client changes due to the current > gate restrictions. You'd be replacing code with calls to identical code. Since SUNWipkg is still a monolithic package, I don't see that it's terribly unsafe. <shrug> Just make sure this gets fixed once the restrictions are lifted. >> - line 95ff: Like depot.py and repositoryconfig.py, this also has a lot >> of hardcoded knowledge of the p5i file specifics. It should really >> all be kept in one place. > > server/depot.py doesn't have the specifics of the format encoded. Just the > specifics about how to construct a publisher and repository object. > > repositoryconfig doesn't have specifics hardcoded either; those are just > repository and publisher attributes and are not specific to the p5i file > format. > > The p5i module, by necessity, must have lots of specifics about publishers > and repositories since certain changes or removals could cause an > incompatible rev in the version of the p5i file. Again, the problem is that you have three different places you need to go to enhance these data structures, without any guidance as to what kind of changes you need to make in each (and that assumes you actually know about all those places in the first place). I'd strongly recommend finding a way to encode all the data you need to know about each of the attributes in a single place, so that all consumers need only look at the attribute metadata to decide how to manipulate the attributes. >> One thing I'm really missing here is the absence of publisher and >> repository uuids. I'm okay with that not coming into this wad, but I >> would dump the alias attribute entirely, since I don't think it's useful >> without uuids. In particular the example server you set up has >> "opensolaris.org" as the name, and "os.org" as the alias. I thought the >> point was that we'd have uuids as the "real" names, and "alias" be the >> human readable name people would see in their "pkg publisher" output, so >> that we could better control the confusion of having client configured >> three publishers publishing the same bits rather than just repointing >> "opensolaris.org" to whatever stream they wanted to be using. > > That doesn't match my understanding of alias and name at all. In fact, the > UUIDs we currently have for a publisher are not to identify the publiser; > they are to identify the image of the client to the publisher. They are > generated (purposefully) on the client side, and that is why the attribute > is called client_uuid. Right; the uuids I'm talking about don't exist. > Furthermore, the name is really the "prefix" of the publisher (such as > opensolaris.org or org.opensolaris) and the alias is a short name that the > user will choose. Okay. I thought we were going to move to uuids for canonical identifiers, provide suggested user-friendly names for them, but allow people to name them as they wished. Nothing consumes any of these attributes yet, right? >> Can we switch to using dashes instead of underscores for text attributes? > > Huh? For example, "refresh_seconds" -> "refresh-seconds". >> A couple of bugs in the test server: >> >> - http://ipkg.sfbay.sun.com:8009/publisher/0/ gives me a file that has >> no terminating newline. > > I didn't add one and json.dump doesn't add it. Should it? I don't know whether it should or not, but if you know that the stream is over, you should make sure that there's a newline at the end. If that's written to a file, it won't have a terminating newline, and that just tends to be a pain in the ass. >> Perhaps future questions: >> >> - should cfg_cache just be the .p5i file instead of having to translate >> between .ini and json formats? > > No, as there is configuration information unrelated to publishers and > repositories stored in cfg_cache. You mean the feed section? > However, my long-term goal is to eventually just store .p5i files in the > repository for the publisher and repository metadata and not have the > information in the cfg_cache at all. So each publisher would be represented by a p5i file in the depot config directory, and the depot would read both those and the cfg_cache file? > I didn't want to do that now as that would mean an even bigger change to > the existing configuration setup. Sure. >> - Where do you see the final configuration store being -- in SMF, or in >> cfg_cache? My dns-sd patch puts more stuff into SMF, but I suspect >> that's not the way you decided to go here at least for convenience. >> But assuming the presence of a Python interface to SMF, would you see >> preferring that? > > I don't think it is practical to store this information in SMF. See above. See what above, exactly? What's impractical about storing this information in SMF, other than we don't yet have a good interface to do so? > Also, the repository needs to be fully usable without SMF, so either way > it needs to be possible for it to be somewhere else if it isn't in SMF. Absolutely. But for the case when we do have SMF, isn't it a better data store? If nothing else, it provides a management interface for the data that we would otherwise have to write. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
