On Wed, Mar 30, 2011 at 09:36:05PM -0700, Brock Pytlik wrote:
> Here's the system repository work that Tim and I have been working
> on for quite a while now.
> http://cr.opensolaris.org/~bpytlik/ips-sysrepo-v1/
>
more comments below.
ed
----------
src/pkg/manifests/package%2Fsysrepo.p5m
src/util/apache2/sysrepo/logs/access_log
src/util/apache2/sysrepo/logs/error_log
- hm. it seems weird to me to deliver empty log files that will get
appended to. (that said, i don't know if this is something that we
recommend to folks packaging up other services.)
----------
src/svc/svc-pkg-sysrepo
src/svc/pkg-sysrepo.xml
- we no longer support 32-bit systems, so you can assume all systems
support 64 bit operation and always start the 64 bit server.
- why are we bothering to support prefork and worker type apache
deployments. can't we just decide on one and use that?
----------
src/svc/svc-pkg-sysrepo
- i'm confused as to the relationship between APACHE_ETC_ROOT,
SYSREPO_TEMPLATE_DIR, and the smf template_dir property. it seems
that we can't actually change the smf template_dir property since we
deliver template files to that location? if so do we really need to
support this as a configurable property in smf? why not just hard
code it in the start method.
- the comment example refers to apache22:
svccfg -s apache22 ...
- i really don't understand why you have to save and restore the
httpd.pid file. (you save and restore it during refresh|stop, but
this script will never run any of the code inbetween the save and
restore in that case.
- this seems kinda weird:
/usr/bin/su pkg5srv
why isn't this whole service running as the pkg5srv user? (also, if
the sysrepo httpd process is writing to the cache dir doesn't it need
to be running as the same user as the htcacheclean process?
- given that we're using apache as an embedded server i don't think we
should be sourcing ${APACHE_ETC_ROOT}/envvars (since it in turn
sources /etc/apache2/2.2/envvars which presumably exists to configure
the the general apache server, not our embedded one.)
----------
src/util/apache2/sysrepo/envvars
- this file doesn't deliver any content and we always update this file
from the smf service start service. so why bother delivering this
file? why not just always overwrite it from the smf start scrit?
----------
src/util/apache2/sysrepo/sysrepo_publisher_response.mako
- wow. this is cryptic. could it get some comments? (i have no idea
what "mako" even is.) do we need a copyright in here?
- the XXX comment says we might not want to expose the source URIs, but
if i access syspub/0 i can see them there anyway as the "proxied_uris"
and "origins". so it seems we're already exposing this information
via additional interfaces? if we have to expose this information then
remove the XXX comment.
----------
etc/pkg/sysrepo/sysrepo_httpd.conf.mako
- this file seems derived from a stock apache template config file but
since we don't have a copy of the original template that you used,
syncing your changes to a future version of apache will be difficult.
do you think that the delta to this file could be factored as a patch
to a reference/unmodified version of the original template file and
then check both in? or just check in a reference version of the
unmodified file?
----------
src/modules/misc.py
- recursive_chown_dir() do you need any special handling for symlinks?
(right now you'll chown them as well but that will actually chown the
target not the link itself.)
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
