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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Pike, Assigned: djc)

Details

Attachments

(1 file, 2 obsolete files)

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.
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.)
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.
Pike: I'll be hacking on pushlog some more today, I'll make sure to fix this bug.
Assignee: nobody → dirkjan
Status: NEW → ASSIGNED
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)
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).
I did a .dump on one of Ted's test databases, and see lines like

INSERT INTO "changesets" VALUES(5, 11, '6a42c9b08be22189856ec006fcb3a68ac814ee61');
.... 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?
I'll check it out. Should work just fine.
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)
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+
Attachment #335689 - Flags: review?(ted.mielczarek)
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+
I agree about 'fromchange'. Yes, repo.lookup() is very quick. I'll update the patch.
Attachment #335689 - Attachment is obsolete: true
Attachment #336317 - Flags: review?(ted.mielczarek)
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.
Attachment #336317 - Flags: review?(ted.mielczarek) → review+
Depends on: 453606
r+ted, deployment bug in 453606.
No longer depends on: 453606
Pushed: http://hg.mozilla.org/users/bsmedberg_mozilla.com/hgpoller/rev/1f5164ca184a
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
Product: Release Engineering → Developer Services
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: