|
|
|
Download charm stats for reporting.
a new ingest pipeline job, completely standalone. downloads stats for
daily delta, so we can do time series analysis against charm popularity.
https://code.launchpad.net/~hazmat/charmworld/charm-stats/+merge/144190
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 4
Patch Set 2 : Download charm stats for reporting. #
Total messages: 5
|
hazmat
Please take a look.
|
12 years, 11 months ago (2013年01月21日 22:02:24 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
lgtm, small suggestions. One larger one to talk about. Since a lot of these jobs are cron'd and kind of forgotten, has there been any thought to adding any sort of email notification or maybe something on the site itself to hint at errors have occurred that need to be addressed? I'm sure it'll be days between issues getting stats from the store and when someone notices and files a bug or something. I suppose we could look at getting webops involved with some nagios hooks or something, but I'd assume setup something simple via email whereever we log an error. https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/common.py File charmworld/jobs/tests/common.py (right): https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/common.py... charmworld/jobs/tests/common.py:10: class RemoteURLBase(MongoTestBase): Can this go into charmworld.testing with the other base test classes? https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... File charmworld/jobs/tests/test_cstat.py (right): https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... charmworld/jobs/tests/test_cstat.py:82: # year roll over There are docstrings on the tests themselves. https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... charmworld/jobs/tests/test_cstat.py:117: def test_delta_calc_on_zero_day(self): missing test? just remove it? https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... charmworld/jobs/tests/test_cstat.py:123: """ just put this up on the first line. It's legit pep257 docstring.
On 2013年01月22日 12:55:07, rharding wrote: > lgtm, small suggestions. > > One larger one to talk about. Since a lot of these jobs are cron'd and kind of > forgotten, has there been any thought to adding any sort of email notification > or maybe something on the site itself to hint at errors have occurred that need > to be addressed? I'm sure it'll be days between issues getting stats from the > store and when someone notices and files a bug or something. that sounds pretty good as a minimal first step, i've added a card. longer term i wanted to have some visibility intro the queue processing from within a management side of the app. on a related topic, while i think celery has some advantages here, i did some more digging on mongodb and celery outside of celery source, and my concerns have some confirmation from the author and community. mongodb and celery https://groups.google.com/forum/?fromgroups=#!searchin/celery-users/mongodb/c... mongodb w/ celery and flower https://groups.google.com/forum/?fromgroups=#!searchin/celery-users/mongodb/c... https://github.com/celery/celery/issues/436 > > I suppose we could look at getting webops involved with some nagios hooks or > something, but I'd assume setup something simple via email whereever we log an > error. yeah.. at the moment hitting the rss feeds and waiting for a time delta greater than sensible is probably the easiest external nagios check. > > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/common.py > File charmworld/jobs/tests/common.py (right): > > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/common.py... > charmworld/jobs/tests/common.py:10: class RemoteURLBase(MongoTestBase): > Can this go into charmworld.testing with the other base test classes? > done. > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... > File charmworld/jobs/tests/test_cstat.py (right): > > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... > charmworld/jobs/tests/test_cstat.py:82: # year roll over > There are docstrings on the tests themselves. yanked, they where my test plan/todo list, obsolete now. > > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... > charmworld/jobs/tests/test_cstat.py:117: def test_delta_calc_on_zero_day(self): > missing test? just remove it? > yanked. > https://codereview.appspot.com/7184044/diff/1/charmworld/jobs/tests/test_csta... > charmworld/jobs/tests/test_cstat.py:123: """ > just put this up on the first line. It's legit pep257 docstring. fixed thanks for the review.