Brock Pytlik wrote:
Shawn Walker wrote:
Brock Pytlik wrote:
client.py:
356: Is there a reason to use the string "known" here instead of a
constant?
No constant exists, and it seemed silly to create a constant for a
single case.
I guess:
state = "known"
followed two lines later by:
state = img.PKG_STATE_INSTALLED
looks strange to me.
But importantly, two lines later with an 'else' in-between :)
The client currently has a very simplified view of things; that is, a
package is 'known' or 'installed'.
Every package that is in the image catalogs is inherently 'known', but
there are additional states beyond that:
PKG_STATE_AVAILABLE -- currently available from a package repository
(i.e. it is in a catalog)
PKG_STATE_INSTALLED -- obvious
Technically, the text here should be 'available' instead of 'known', but
I didn't want to deal with re-working the output of pkg list yet, and
wanted to do that as a separate project. That's also why internally
PKG_STATE_AVAILABLE is used, as I believe that expresses the true state
of the package clearly.
In the near future, packages will also have additional states beyond
that, such as PKG_STATE_FROZEN, etc. For example, you could have a
package that is PKG_STATE_AVAILABLE, PKG_STATE_INSTALLED and
PKG_STATE_FROZEN.
I've implemented the state recording mechanisms to ensure that even if a
package is no longer PKG_STATE_AVAILABLE, the remaining states will stay
intact.
Also, does img.PKG_STATE_INSTALLED even exist anymore? It kinda looks
like it's been moved/removed.
No, it exists, and is used; it's just a class constant now instead of a
module constant. Look at set_pkg_state or various other code in image.py.
publisher.py:
946: why do the import here?
Avoids the overhead of importing the module until it's needed.
We should probably decide whether we want to start doing this or not. I
think, so far, we've gone with a "import everything at the top"
approach. I brought this up to dp last week and he believed it wouldn't
make much of a difference in terms of performance. (I haven't had the
time to do the performance testing myself.) I'd suggest that if we're
going in this direction, we make a decision to do it as a team, and look
to make those changes as we touch bits of code. Otherwise, I don't see a
reason that this needs special treatment. If the module is somehow
special, please explain why :)
It isn't, but considering we've gotten some cases down to sub
one-second, avoiding any unnecessary module imports until we actually
need them seems useful. I'm willing to defer to the majority view here,
but I would prefer to start shifting to the "don't import until you need
it" model to help reduce our load times (and memory usage!) within
reason of course.
I realise there's a hidden argument about knowing about missing import
dependencies at the start rather than not until some function is called...
manifest.py:
481-482: why not just compare signatures with self.signatures?
Because that would be a self-fulfilling prophecy :)
The signatures are generated based on the current contents, so
validate would always be true.
This allows an external caller with the signature data they have for
the manifest to validate easily against what the manifest thinks its
signatures are.
Sorry, let me be more explicit why is this code:
481 + old_signatures = self.signatures
482 + if signatures != old_signatures:
483 + raise
api_errors.BadManifestSignatures(self.fmri)
Different from this code:
482 + if signatures != selfg.signatures:
483 + raise
api_errors.BadManifestSignatures(self.fmri)
Ah; leftover. Will change.
server/catalog:
We could probably just remove the checks for .pag and .dir? Those
haven't existed since the 2008.05 release.
1151: what are the possible classes that could be coming in here?
It's marked as a classmethod now so it can access __parse_entry.
Ok, I'll try again. Why is it a static method instead of a class method?
I'll assume you meant why is it a @classmethod instead of a
@staticmethod since it is a @classmethod right now :)
In that case, yes, I could change it to that, but then I would have to
call ServerCatalog.__parse_entry instead and I don't care for that form
since it embeds the class name in the code.
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
