WIP: Cancel message by LipuFei · Pull Request #370 · Tribler/dispersy

LipuFei · 2014-09-09T12:01:07Z

Use cancel message to replace undo message.
All undo message code remain the same.

LipuFei · 2014-09-09T12:57:21Z

@whirm I'll remove the pep8 commit for now, otherwise the PR is too difficult to review.

tribler-ci · 2014-09-09T13:08:54Z

Test FAILED.
Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/784/

whirm · 2014-09-09T13:11:30Z

@LipuFei yes, those should always go alone

NielsZeilemaker · 2014-09-10T15:21:18Z

@LipuFei why are you adding the cancel to the permission triplet? That shouldn't be neccesary. You authorise someone to send a dispersy_cancel message etc.

LipuFei · 2014-09-10T15:24:09Z

@NielsZeilemaker I made this PR to make it easy for us to review. Thanks! I'll check everything again.

LipuFei · 2014-09-10T15:36:44Z

@NielsZeilemaker What does the permission_triplets mean?

NielsZeilemaker · 2014-09-10T15:40:11Z

Basically, you can authorize/revoke or grant someone the permission to create a message of a certain type. I wouldn't start messing with this as its never tested/understood by me.
Moreover, are you sure you want to introduce a cancel callback? Maybe calling the undo callback would suffice. Chaning it slightly to allow you to pass a cancelled/undone/done variable instead of the undone/done currently used.

NielsZeilemaker · 2014-09-10T15:41:40Z

Btw, the documentation regarding the permissions should still be correct. You can have a look there.

LipuFei · 2014-09-10T15:44:09Z

@NielsZeilemaker I had thought about it and I decided to duplicate the undo stuff instead of messing them up. Later, when we want to remove the undo, we can simply remove every undo-related thing without worry about causing damage to the cancel message. I will read the permission docs. Thanks!

NielsZeilemaker · 2014-09-10T15:47:23Z

I would remove the cancel here, but leave it in timeline.py. E.g. add it when the "undo" thingy is there.

tribler-ci · 2014-09-10T16:39:00Z

Test FAILED.
Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/785/

tribler-ci · 2014-09-10T17:29:00Z

Test FAILED.
Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/786/

tribler-ci · 2014-09-10T17:34:00Z

Test FAILED.
Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/787/

LipuFei · 2014-09-10T19:09:47Z

@whirm I guess this is more or less it. I will check the unit tests one more time after my presentation rehearsal. Also, before we merge, we should change the AllChannel community to use this cancel message and test on jenkins.

whirm · 2014-09-11T10:17:38Z

Ok, let's review it later

tribler-ci · 2015-01-21T13:13:03Z

Merged build triggered.

tribler-ci · 2015-01-21T13:13:03Z

Merged build started.

tribler-ci · 2015-01-21T13:23:29Z

Merged build finished.

tribler-ci · 2015-01-21T13:23:30Z

Test FAILed.
Refer to this link for build results: http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/836/

tribler-ci · 2015-01-21T13:28:03Z

Merged build triggered.

tribler-ci · 2015-01-21T13:28:03Z

Merged build started.

tribler-ci · 2015-01-21T13:38:30Z

Merged build finished.

tribler-ci · 2015-01-21T13:38:30Z

Test FAILed.
Refer to this link for build results: http://jenkins.tribler.org/job/GH_Dispersy_pull-request-tester_devel/837/

tribler-ci · 2015-03-10T10:18:17Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/848/
Test FAILed.

LipuFei · 2015-04-07T09:31:03Z

@NielsZeilemaker One of the cancel message tests didn't pass. The test does the following:

Three nodes: SELF, NODE, OTHER
What they do:
1. SELF gives NODE the permission to cancel.
2. OTHER created message M1.
3. NODE cancels M1 with cancel message C1.
4. M1 should be found as cancelled by NODE (This one passed).
5. SELF revokes NODE's cancel permission.
6. M1 should still be marked as cancelled by NODE (This one failed).

This test was passing before. Am I doing it correctly? If a user's cancel permission has been revoked, should his old cancel messages be regarded as invalid as well?

NielsZeilemaker · 2015-04-07T10:25:20Z

@LipuFei maybe create a wiki page describing the cancel message, and what it should do. Then afterwards, I can have a look and maybe we can modify the behaviour of your implementation accordingly.
Also describe situations such as the one above. A gets the permission to cancel other, A's permission gets revoke -> what should be do with the messages cancelled by A.

LipuFei · 2015-04-07T11:57:56Z

@NielsZeilemaker I've made a wiki page: https://github.com/Tribler/dispersy/wiki/Cancel-Message. Hope it explains.

NielsZeilemaker · 2015-04-07T20:07:33Z

@LipuFei I made some changes to the document. I guess cancel messages should also not cancel an authorize message right? Or are we adding that? Additionally, we should think about being able to drop cancelled messages from the database in order to save space.

tribler-ci · 2015-05-19T13:46:45Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/864/
Test FAILed.

tribler-ci · 2015-06-18T13:25:17Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/871/
Test FAILed.

tribler-ci · 2015-07-21T14:00:26Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/886/
Test FAILed.

tribler-ci · 2015-08-19T09:45:32Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/891/
Test FAILed.

tribler-ci · 2015-08-31T09:40:33Z

Refer to this link for build results (access rights to CI server needed):
http://jenkins.tribler.org//job/GH_Dispersy_pull-request-tester_devel/894/
Test FAILed.

LipuFei · 2015-09-14T16:59:35Z

Hi @NielsZeilemaker, could you review this PR again? Thanks!

LipuFei force-pushed the cancel_message branch from 9d574c6 to 4e5b7ef Compare September 9, 2014 12:57

NielsZeilemaker reviewed Sep 10, 2014
View reviewed changes

LipuFei force-pushed the cancel_message branch from 4e5b7ef to d84e09b Compare September 10, 2014 16:27

LipuFei force-pushed the cancel_message branch 2 times, most recently from 9d0f83b to b6cd526 Compare September 10, 2014 17:18

LipuFei force-pushed the cancel_message branch from b6cd526 to 162c809 Compare January 21, 2015 13:09

LipuFei force-pushed the cancel_message branch from 5b24637 to d8fe21f Compare May 19, 2015 13:28

LipuFei force-pushed the cancel_message branch from d8fe21f to 2f95ae6 Compare June 18, 2015 13:08

LipuFei force-pushed the cancel_message branch from 2f95ae6 to 1ced941 Compare July 16, 2015 13:39

LipuFei force-pushed the cancel_message branch from 1ced941 to 475c87b Compare August 19, 2015 09:30

LipuFei added 13 commits August 31, 2015 11:23

Add CancelPayload

e8eda4f

Add cancel messages for community

94a2d6b

Add cancel message unit tests

92cc3a0

Add cancel_callback for message

6522363

Add cancel message check in timeline.py

45d0ff1

Add cancel message for conversion.py

8e5e508

fix some comments

758945b

add cancel message check

e7bd22f

fix cancel_message

d51476a

fix test_cancel

484a0e2

fix a typo bug, correct some comments, and add some TODO comments

7f16126

some fixes

6c25383

fix cancel_message unit test

1709e3c

LipuFei force-pushed the cancel_message branch from 475c87b to 1709e3c Compare August 31, 2015 09:23

whirm changed the title ~~Cancel message~~ WIP: Cancel message Apr 29, 2016

Uh oh!

Conversation

LipuFei commented Sep 9, 2014

Uh oh!

LipuFei commented Sep 9, 2014

Uh oh!

tribler-ci commented Sep 9, 2014

Uh oh!

whirm commented Sep 9, 2014

Uh oh!

NielsZeilemaker commented Sep 10, 2014

Uh oh!

LipuFei commented Sep 10, 2014

Uh oh!

LipuFei commented Sep 10, 2014

Uh oh!

NielsZeilemaker commented Sep 10, 2014

Uh oh!

NielsZeilemaker commented Sep 10, 2014

Uh oh!

LipuFei commented Sep 10, 2014

Uh oh!

NielsZeilemaker Sep 10, 2014

Choose a reason for hiding this comment

Uh oh!

tribler-ci commented Sep 10, 2014

Uh oh!

tribler-ci commented Sep 10, 2014

Uh oh!

tribler-ci commented Sep 10, 2014

Uh oh!

LipuFei commented Sep 10, 2014

Uh oh!

whirm commented Sep 11, 2014

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Jan 21, 2015

Uh oh!

tribler-ci commented Mar 10, 2015

Uh oh!

LipuFei commented Apr 7, 2015

Uh oh!

NielsZeilemaker commented Apr 7, 2015

Uh oh!

LipuFei commented Apr 7, 2015

Uh oh!

NielsZeilemaker commented Apr 7, 2015

Uh oh!

tribler-ci commented May 19, 2015

Uh oh!

tribler-ci commented Jun 18, 2015

Uh oh!

tribler-ci commented Jul 21, 2015

Uh oh!

tribler-ci commented Aug 19, 2015

Uh oh!

tribler-ci commented Aug 31, 2015

Uh oh!

LipuFei commented Sep 14, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!