Feature #9062

Migrate otr-bot.py to python-potr

Added by anonym 2015-03-16 15:44:29 . Updated 2015-03-22 11:58:05 .

Status:
Resolved
Priority:
Normal
Assignee:
anonym
Category:
Test suite
Target version:
Start date:
Due date:
% Done:

100%

Feature Branch:
test/9062-otr-bot-with-python-potr
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

python-otr is not in Debian Jessie, but python-potr is in both Jessie and wheezy-backports.

Also, python-potr requires some new (binary, so unfriendly to our test suite’s local.yml) format for the OTR keys, but includes a script to convert from the old format to the new one. Unfortunately the Debian package seem to be lacking this script, which seems like a bug (it’s explicitly mentioned that the package includes the script, and it has the man page).


Subtasks


History

#1 Updated by anonym 2015-03-16 15:45:52

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 20
  • QA Check set to Dev Needed
  • Feature Branch set to test/9062-otr-bot-with-python-potr

Pushed a PoC.

#2 Updated by intrigeri 2015-03-16 16:18:12

> * Target version: Tails_4.0

This seems quite more urgent than 4.0 to me, as it’ll be blocking Feature #8165.
Perhaps this should simply block Feature #8001?

#3 Updated by anonym 2015-03-16 17:17:08

#4 Updated by anonym 2015-03-16 17:17:15

  • Target version changed from Tails_2.0 to Tails_1.3.2

intrigeri wrote:
> > * Target version: Tails_4.0
>
> This seems quite more urgent than 4.0 to me, as it’ll be blocking Feature #8165.

True. FTR, I’ve reported a bug about this. Will post the bug number once I learn it.

> Perhaps this should simply block Feature #8001?

ok, added… :|

#5 Updated by anonym 2015-03-16 17:21:43

anonym wrote:
> FTR, I’ve reported a bug about this. Will post the bug number once I learn it.

Here we go: Debian bug #780610

#6 Updated by anonym 2015-03-16 17:28:55

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Dev Needed to Info Needed

intrigeri, I’d still appreciate a review/test of the current state of this branch ASAP, so I just have to rewrite commit:eb4abc9 to use the convertkey.py from the package, once it’s fixed in Debian. commit:5a32a87 is the key thing.

#7 Updated by intrigeri 2015-03-16 20:23:57

anonym wrote:
> Here we go: Debian bug #780610

Great, thanks! I’ve usertagged it “test-suite”.

#8 Updated by intrigeri 2015-03-16 20:36:36

  • Assignee changed from intrigeri to anonym
  • % Done changed from 20 to 30
  • QA Check changed from Info Needed to Dev Needed

anonym wrote:
> intrigeri, I’d still appreciate a review/test of the current state of this branch ASAP,

Congrats! I did a very rough code review (I didn’t go as far as reading the potr API) and my only comment is that in getPolicy, we return True and then do other things: I don’t understand what this is about.

Once this is fixed:

  • please a ticket (type of work = Wait) so that we think of removing convertkey.py once it’s in a version of the Debian package that we can require for the test suite;
  • make the “XXX” that was added to chatbot_helper.rb point to the relevant Debian bug report;
  • go wild, merge into test/8001-8002-pidgin-xmp-otr, and close this ticket? (we already have two tickets tracking that branch, perhaps a third one won’t help understanding where things are at) — if you think I should test this before it’s merged into test/8001-8002-pidgin-xmp-otr, just say so.

#9 Updated by intrigeri 2015-03-16 20:36:49

  • blocked by deleted (Feature #8001: Automatic tests for OTR in Pidgin)

#10 Updated by intrigeri 2015-03-16 20:36:58

#11 Updated by intrigeri 2015-03-16 21:21:51

intrigeri wrote:
> if you think I should test this before it’s merged into test/8001-8002-pidgin-xmp-otr, just say so.

I’ve successfully run pidgin.feature after merging this branch into test/8001-8002-pidgin-xmp-otr, and in turn into current stable.

#12 Updated by anonym 2015-03-17 09:27:01

  • Assignee changed from anonym to intrigeri
  • QA Check changed from Dev Needed to Info Needed

intrigeri wrote:
> anonym wrote:
> > intrigeri, I’d still appreciate a review/test of the current state of this branch ASAP,
>
> Congrats! I did a very rough code review

Thanks!

> (I didn’t go as far as reading the potr API)

Heads up: there’s no API documentation, or documentation whatsoever. I had to backwards engineer the API by reading the code. I found a blog article that was really helpful, even though it got some things wrong (or at least it made some things awkward), from what I understand. FWIW.

> and my only comment is that in getPolicy, we return True and then do other things: I don’t understand what this is about.

Ooops, that was some left-over code from an experiement. Deleted.

> Once this is fixed:
>
> * please a ticket (type of work = Wait) so that we think of removing convertkey.py once it’s in a version of the Debian package that we can require for the test suite;

Bug #9066

> * make the “XXX” that was added to chatbot_helper.rb point to the relevant Debian bug report;

I think I’ve made it clear elsewhere that the commit that added that “XXX” etc. should be rewritten, so I’ll skip this.

> * go wild, merge into test/8001-8002-pidgin-xmp-otr, and close this ticket? (we already have two tickets tracking that branch, perhaps a third one won’t help understanding where things are at) — if you think I should test this before it’s merged into test/8001-8002-pidgin-xmp-otr, just say so.

Well, I don’t see this ticket’s branch as complete. I want to rewrite its history, and merging stuff into it will make it a pain. The other branch is, I guess, finished now, so we can forget about it and Feature #8001 and Feature #8002 until Bug #9066 is resolved. Ok?

intrigeri wrote:
> intrigeri wrote:
> > if you think I should test this before it’s merged into test/8001-8002-pidgin-xmp-otr, just say so.
>
> I’ve successfully run pidgin.feature after merging this branch into test/8001-8002-pidgin-xmp-otr, and in turn into current stable.

Great!

#13 Updated by intrigeri 2015-03-17 09:46:11

>> * go wild, merge into test/8001-8002-pidgin-xmp-otr, and close this ticket? (we
>> already have two tickets tracking that branch, perhaps a third one won’t help
>> understanding where things are at) — if you think I should test this before it’s
>> merged into test/8001-8002-pidgin-xmp-otr, just say so.

> Well, I don’t see this ticket’s branch as complete. I want to rewrite its history, and merging stuff into it will make it a pain. The other branch is, I guess, finished now, so we can forget about it and Feature #8001 and Feature #8002 until Bug #9066 is resolved. Ok?

This could work, sure.

But this will make the transition to Jessie as the testing environment a bit more painful (the changes needed to make this work won’t be as minimal as expected, and then it can be harder to have it merged into all branches we want to test, which needs to happen at the same time as we declare Jessie to be the new supported environment). So IMO, to save some future hassle, it’s worth merging this branch now, after minimally rewriting the commit that introduces convertkey.py to make it clear that it won’t be rewritten but instead will be superseded once Bug #9066 is fixed. Other than Git history prettiness, any reason why we shouldn’t do that?

#14 Updated by intrigeri 2015-03-17 09:49:22

  • Assignee changed from intrigeri to anonym

#15 Updated by anonym 2015-03-17 11:32:42

  • QA Check deleted (Info Needed)

intrigeri wrote:
> >> * go wild, merge into test/8001-8002-pidgin-xmp-otr, and close this ticket? (we
> >> already have two tickets tracking that branch, perhaps a third one won’t help
> >> understanding where things are at) — if you think I should test this before it’s
> >> merged into test/8001-8002-pidgin-xmp-otr, just say so.
>
> > Well, I don’t see this ticket’s branch as complete. I want to rewrite its history, and merging stuff into it will make it a pain. The other branch is, I guess, finished now, so we can forget about it and Feature #8001 and Feature #8002 until Bug #9066 is resolved. Ok?
>
> This could work, sure.
>
> But this will make the transition to Jessie as the testing environment a bit more painful (the changes needed to make this work won’t be as minimal as expected, and then it can be harder to have it merged into all branches we want to test, which needs to happen at the same time as we declare Jessie to be the new supported environment). So IMO, to save some future hassle, it’s worth merging this branch now, after minimally rewriting the commit that introduces convertkey.py to make it clear that it won’t be rewritten but instead will be superseded once Bug #9066 is fixed. Other than Git history prettiness, any reason why we shouldn’t do that?

Makes sense (my aversion for messing up the Git history isn’t very rational, I admit). The commit has been rewritten, and the branch force pushed and merged into test/8001-8002-pidgin-xmp-otr. So, let’s merge test/8001-8002-pidgin-xmp-otr, right?

#16 Updated by intrigeri 2015-03-17 11:46:23

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Resolved this child ticket. Let’s handle what’s left (if any) in Feature #8001.

#17 Updated by BitingBird 2015-03-22 11:58:06

  • Target version changed from Tails_1.3.2 to Tails_1.3.1