Design decision: use bzr commit --local ?

Bug #240670 reported by Daniel Holbach
4
Affects Status Importance Assigned to Milestone
five-a-day
Fix Released
Medium
Unassigned

Bug Description

I'm pondering using
  bzr commit --local
for the cases where we don't push to the branch (after one hour of not pushing).

I realise that the code path will be a bit longer for local commits but it has obvious advantages:
 1) less conflicts because of using 5-a-day on various machines (imagine two .5-a-day-data directories with unpushed changes)
 2) commit --local is guaranteed to work and if the push (or 'commit' in this case) fails, we have a local state that makes sense

Can you let me know what you think?

Changed in five-a-day:
importance: Undecided → Medium
status: New → Confirmed
Revision history for this message
Markus Korn (thekorn) wrote :

Well, this bugreport is surprising me. I always thought that 5-a-day is doing a local commit if the last commit was done by the same user within one hour, but obviously I'm wrong.
I'm not using bound branches that often, so I also don't have much experiences or knowledge about local commits, but I can't see any cons against using 'commit --local' in this cases.

Markus

Revision history for this message
Daniel Holbach (dholbach) wrote :

I totally forgot to mention how I came to this idea:
 - user has not added SSH keys to LP
or
 - user is not member of the 5-a-day team

In those cases 5-a-day will try to commit and give a nasty warning. After figuring out what went wrong, the users fixes it and tries to submit the same bug again: 5-a-day won't take accept the bug because it's in the file already. :-)

Daniel Hahler: what do you think?

Revision history for this message
Daniel Hahler (blueyed) wrote : Re: [Bug 240670] Re: Design decision: use bzr commit --local ?

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

> Daniel Hahler: what do you think?

I like the idea, but it probably needs adjustments to the "committed in
last 60 minutes" check, because the local commit probably resets the
timer then.

- --
http://daniel.hahler.de/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIWXbzfAK/hT/mPgARAuFUAJ9505YopHwFETXlshwYT5+QVXwxXgCfZNpx
aLGH23wTDx+m+M2/pPHedLc=
=lpfX
-----END PGP SIGNATURE-----

Revision history for this message
Daniel Holbach (dholbach) wrote :

I just talked to Robert Collins. A short discussion revealed that the only good fix to the problem is having per-user branches.

Revision history for this message
Daniel Holbach (dholbach) wrote :

Can you please check out 0.43 (not released yet) and see if it works for you and makes sense?

  * bzr.py, files.py: users push to their own branches, every 10th commit is
    pushed.

Revision history for this message
Markus Korn (thekorn) wrote :

This happens when I run 5-a-day for the first time:

$ ./5-a-day --add 123456
Please type in:
        echo <Your Launchpad ID> > ~/.5-a-day
markus@thekorn:~/devel/5-a-day/ubuntu$ echo thekorn > ~/.5-a-day
markus@thekorn:~/devel/5-a-day/ubuntu$ ./5-a-day --add 123456
bzr: ERROR: Not a branch: "http://bazaar.launchpad.net/~thekorn/5-a-day-data/main/".
Committing to: /home/markus/.5-a-day-thekorn/
bzr: ERROR: no changes to commit. use --unchanged to commit anyhow
Created new branch.
bzr: ERROR: No pull location known or specified.
Committing to: /home/markus/.5-a-day-thekorn/
bzr: ERROR: no changes to commit. use --unchanged to commit anyhow
bzr: ERROR: No location specified or remembered
bzr failed with error code 768
bzr failed with error code 768
markus@thekorn:~/devel/5-a-day/ubuntu$ ./5-a-day --add 123456
Bug '123456' has already been added today. Aborting.
markus@thekorn:~/devel/5-a-day/ubuntu$ ls /home/markus/.5-a-day-thekorn/
data
markus@thekorn:~/devel/5-a-day/ubuntu$ cat /home/markus/.5-a-day-thekorn/data
1213871107 123456
markus@thekorn:~/devel/5-a-day/ubuntu$

So maybe the initial creation of the data branch for the user on launchpad.net has do be done.
I'm not sure if the other bzr errors are related to the fact that there is n branch already created for me.

Markus

Revision history for this message
Daniel Holbach (dholbach) wrote :

I think I fixed a number of issues. Thanks for testing - can you try again?

Revision history for this message
Markus Korn (thekorn) wrote :

No, the initial problem is not solved:
As long as the users branch (like: http://bazaar.launchpad.net/~thekorn/5-a-day-data/main/) on launchpad not exist you will always get the errors described above.

Will try to debug this more tomorrow morning

Markus

Revision history for this message
Daniel Holbach (dholbach) wrote :

I doubt it. Can you delete your branch, wait a bit, then try again? https://code.launchpad.net/~thekorn/5-a-day-data/main/+delete

The 5-a-day code in question:

def checkout_branch():
    readonly = "http://"+remote_branch().split("@")[1]
    try:
        branch.Branch.open_containing(readonly)
    except errors.NotBranchError:
        err = setup_local_branch()
        return err
    err = os.system("bzr branch %s %s" % (readonly, files.local_branch()))
    return err

Revision history for this message
Daniel Holbach (dholbach) wrote :

Now I changed the team file to live in the per-user branch too.

Daniel: does the new behaviour work for you?

Markus: can you test if you will need to change the applet? files.modify_teams(list-of-teams) should do the work for you now.

Revision history for this message
Daniel Holbach (dholbach) wrote :

Did some changes: try files.add_to_teams(teams) and files.remove_from_teams(teams) instead. :-)

Revision history for this message
Markus Korn (thekorn) wrote :

ok, I tested this changes again, this is working for me so far.
I will look at the applet over the weekend.

I'm now sure of a periode of local commit until one remote commit makes sense. Ths means if a user does "only" 5 bugs a day, he will wait at least two days until his bugs make it into the statistic, this seems to be a bit long to me. Reducing this number is also not a solution as this won't reduce the bzr-actions for power-users significantly.

Maybe an additional date based check would be a solution, two possibilities came in my mind:
1.) branch.Branch.open() the remote branch, check for the date of the last rev there and use this date to calculate the time since last remote commit.
2.) put timestamp of last commit in a ~/.5-a-day-lastcommit, not as save as 1.) but reduce remote bzr actions

Another thing is: what about allowing to run ./5-a-day --force without a bugnumber, so if a user really wants to remote-commit the changes it is possible without committing a new bug.

Markus

Revision history for this message
Daniel Holbach (dholbach) wrote :

 - Running 5-a-day --add -f is already supported.
 - I personally thought it'd consume too much time - seems it doesn't:
>>> import time; from bzrlib import branch
>>> t = time.time(); b = branch.Branch.open("http://bazaar.launchpad.net/~dholbach/5-a-day-data/main"); lastrev = b.last_revision(); ts = b.repository.get_revision(lastrev).timestamp; print time.time()-t
1.73250198364
>>>

Revision history for this message
Daniel Holbach (dholbach) wrote :

Try out the attached patch - it definitely feels slower.

Revision history for this message
Daniel Holbach (dholbach) wrote :

... which with the applet shouldn't be a problem. Just on the command line it feels a bit weird.

Revision history for this message
Daniel Holbach (dholbach) wrote :

Maybe with the new "everybody pushes to their own branch" behaviour we should always push. *shrug*

Revision history for this message
Markus Korn (thekorn) wrote :

Well patch in comment 14 uses bzr+ssh:// and in your timing you are using http:// - this explains why it's slower

In [2]: t = time.time(); b = branch.Branch.open("http://bazaar.launchpad.net/~dholbach/5-a-day-data/main"); lastrev = b.last_revision(); ts = b.repository.get_revision(lastrev).timestamp; print time.time()-t
1.55852007866

In [3]: t = time.time(); b = branch.Branch.open("bzr+ssh://<email address hidden>/~thekorn/5-a-day-data/main"); lastrev = b.last_revision(); ts = b.repository.get_revision(lastrev).timestamp; print time.time()-t
6.10737895966

so either always push (that won't hurt that much, everybody has his own branch) or use http://

Markus

Revision history for this message
Daniel Holbach (dholbach) wrote :

Using 'http://' for now.

Changed in five-a-day:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.