Closed
Bug 449381
Opened 16 years ago
Closed 16 years ago
support "tip" and "default" in pushlog?fromchange=...&tochange=tip
Categories
(Developer Services :: Mercurial: hg.mozilla.org, defect)
Developer Services
Mercurial: hg.mozilla.org
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Pike, Assigned: djc)
Details
Attachments
(1 file, 2 obsolete files)
4.20 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
It'd be nice to be able to get all changes after a particular changeset. One way to do that would be to specify "tip" as tochange, which is probably a good thing to do anyway. I guess "default" should behave the same way. The other way would be to make the tochange= argument optional. I guess doing both would be cool, even. Maybe make "tip" the default value for tochange, that should be good enough.
Reporter | ||
Comment 1•16 years ago
|
||
djc, could you confirm that we just need to apply the following to both tochange and fromchange in pushlog-feed.py around lines 134f? from mercurial.node import hex def lookup(changespec): return hex(web.repo.lookup(changespec)) (I didn't find a nicer way to get back to hex than to import it from node.)
Reporter | ||
Comment 2•16 years ago
|
||
PS: AFAICT, that should even include tags then, so we could do things like "get all changes since FIREFOX_3_1_0_RELEASE" or somesuch.
Assignee | ||
Comment 3•16 years ago
|
||
Pike: I'll be hacking on pushlog some more today, I'll make sure to fix this bug.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → dirkjan
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•16 years ago
|
||
This is the adhoc patch, untested even. I didn't add any error reporting, as there isn't any error reporting for any other error so far, if you pass in junk, it'll raise an mercurial.repo.RepoError: unknown revision 'junk'. This seems to be the least cost route to cause less load on hg from our pollers, see bug 452102.
Attachment #335541 -
Flags: review?(ted.mielczarek)
Attachment #335541 -
Flags: review?(dirkjan)
Assignee | ||
Comment 5•16 years ago
|
||
Hmm, yeah, sorry for taking so long on this. I think your patch is mostly fine, but need to check if pushlog keeps full id's (hex) or shortened versions (short).
Reporter | ||
Comment 6•16 years ago
|
||
I did a .dump on one of Ted's test databases, and see lines like INSERT INTO "changesets" VALUES(5, 11, '6a42c9b08be22189856ec006fcb3a68ac814ee61');
Reporter | ||
Comment 7•16 years ago
|
||
.... thinking about this, I guess we can simplify getpushlogentriesbychangeset after doing lookup: fromchange += "%" e = conn.execute("SELECT pushid FROM changesets WHERE node LIKE ?", (fromchange,)).fetchone() should become e = conn.execute("SELECT pushid FROM changesets WHERE node == ?", (fromchange,)).fetchone() Is there a way to test this without a website running?
Assignee | ||
Comment 8•16 years ago
|
||
I'll check it out. Should work just fine.
Assignee | ||
Comment 9•16 years ago
|
||
This is my attempt at this. It gives the current time for empty feeds (as there are no csets to get the time from), and picks the null revision as the default fromchange value. That's not necessarily effective, though, as that one is probably not in the pushlog db. So I'm not sure what a good default for the fromchange would be. I've used "default" as the tochange default, I think that makes more sense than tip in the face of named branches.
Attachment #335541 -
Attachment is obsolete: true
Attachment #335541 -
Flags: review?(ted.mielczarek)
Attachment #335541 -
Flags: review?(dirkjan)
Reporter | ||
Comment 10•16 years ago
|
||
Comment on attachment 335689 [details] [diff] [review] Patch to add sane defaults and run params through repo lookup. r=me. Ted?
Attachment #335689 -
Flags: review+
Reporter | ||
Updated•16 years ago
|
Attachment #335689 -
Flags: review?(ted.mielczarek)
Comment 11•16 years ago
|
||
Comment on attachment 335689 [details] [diff] [review] Patch to add sane defaults and run params through repo lookup. + elif 'fromchange' in req.form or 'tochange' in req.form: + fromchange = hex(repo.lookup(req.form.get('fromchange', ['null'])[0])) I think we should make 'fromchange' required. I don't see a good use case for letting people query everything from the first changeset on. Is repo.lookup expensive, or will this stay quick? Looks good to me otherwise.
Attachment #335689 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 12•16 years ago
|
||
I agree about 'fromchange'. Yes, repo.lookup() is very quick. I'll update the patch.
Assignee | ||
Comment 13•16 years ago
|
||
Attachment #335689 -
Attachment is obsolete: true
Attachment #336317 -
Flags: review?(ted.mielczarek)
Reporter | ||
Comment 14•16 years ago
|
||
Can we get this reviewed and deployed? I'm really hopeful that we can see significantly less load from our pollers if we can search by range.
Updated•16 years ago
|
Attachment #336317 -
Flags: review?(ted.mielczarek) → review+
Comment 16•16 years ago
|
||
Pushed: http://hg.mozilla.org/users/bsmedberg_mozilla.com/hgpoller/rev/1f5164ca184a
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•10 years ago
|
Product: Release Engineering → Developer Services
You need to log in
before you can comment on or make changes to this bug.
Description
•