Add match results dialog #17

nirizr · 2016-08-16T23:37:44Z

Remaining tasks:

~~Convert MatchResultsDialog to a dock-able form.~~ - CBA, not important enough.
Replace web display with a drawn graph as an additional dockable form.
Handle cancel during the results download stage, currently network paged support does not support canceling.
Protect user opening multiple match dialogs.

Signed-off-by: Nir Izraeli nirizr@gmail.com

nirizr · 2016-08-16T23:38:36Z

This if far from functional, has trivial errors atm.

shiftre · 2016-09-19T00:48:08Z

Uh, I only skimmed it, (along with the first time), but this one should be merged only after match backend is inside, am i wrong ?

nirizr · 2016-09-19T03:36:58Z

You're right. It's far from being ready

which proved to be quite slow

+ performance optimizations and some debugging Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Also, small refactor of QueryWorker and surrounding functions Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Replace delayed_query calls which potentially risked double starting a query worker (and hang). Replace delayed_worker calls with calling query worker's start method Simplify network delayed query interface Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Plus better exception logging Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

…caching them in MatchAction Signed-off-by: Nir Izraeli <nirizr@gmail.com>

…reshold is reached * Avoid exception and remove related exception handling code * Upload serialized instances when all functions were serialized but didn't reach upload threshold Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

shiftre · 2017-02-25T21:33:25Z

idaplugin/rematch/actions/match.py

+    func = instances.FunctionInstance(self.file_version_id, offset)
+    self.instance_set.append(func.serialize())
+
+    if len(self.instance_set) >= 100 or not self.functions:


You're comparing self.functions twice here, in different ways, the 2nd condition will never be met.

I don't understand this condition.

If there are no functions, return.
I also don't understand the actual condition of comparing the value to 100 and only
then uploading, what happens if there are 10?

The idea here is this:
We should send objects to the server in two cases:

We collected 100 instances, enough to send a request for. We'd like to consolidate multiple requests into few requests with decent size.

If we just processed the last function (we poped the last one, and self.functions just became empty). We need to make sure we won't "forget" the last items if they didn't reach the bulk-size.

I know this piece of code is a bit too difficult to understand. Any ideas on how to make it clearer?

I'll also comment it so it'll be clearer.

shiftre · 2017-02-25T22:33:35Z

idaplugin/rematch/actions/match.py

-    self.cancel_upload()
+    self.clean()
+    self.delayed_queries = []
+
    self.start_task()

  def start_task(self):


I'm writing a note here but the note refers to something which is already in master.
the self.source variable does is incomplete in the conditions of 'range'and source_single.

I'm not sure I follow. Is there something to fix here? Is there something to fix for master? Would you like me to rebase again and you'll comment then?

shiftre · 2017-02-25T22:35:10Z

idaplugin/rematch/actions/match.py

@@ -171,24 +182,86 @@ def perform_task(self):
      progress = int(r['progress'])
      status = r['status']
      if status == 'failed':
-        self.pbar.reject()
+        self.pbar.cancel()


How about adding some debugging or logging for a task which failed?
We'd most likely catch it earlier, but if anything is higher (nothing transport side), we'd miss it
and won't have all tasks synced.

Yes. This is quite big and not really logged. I'll add logging throughout the code. Good idea!

shiftre · 2017-02-25T22:48:21Z

idaplugin/rematch/network.py

+        if not self.paginate:
+          break
+
+        if 'next' not in response or not response['next']:


Wouldn't the 2nd condition fail in case we get a str response? this should separated to list / dict and str

self.paginate should be only true for json responses. For those response is a dictionary (edit: or a list).

What do you think of a test to make sure self.json == True and that response is a dictionary?

* more logs and a few additional comments * validate pagination network support Signed-off-by: Nir Izraeli <nirizr@gmail.com>

nirizr · 2017-02-27T15:38:10Z

🎉 😀 🎉

nirizr added the do not merge label Aug 16, 2016

nirizr added this to the first-release milestone Aug 16, 2016

nirizr force-pushed the nirizr/match_results_dialog branch 2 times, most recently from 144b04d to 6144b31 Compare August 25, 2016 23:26

shiftre added the question label Sep 19, 2016

nirizr removed the question label Sep 19, 2016

nirizr force-pushed the nirizr/match_results_dialog branch 13 times, most recently from 9d9c338 to 1247cd1 Compare November 3, 2016 03:14

nirizr force-pushed the nirizr/match_results_dialog branch from 1247cd1 to 7e01c33 Compare November 8, 2016 02:16

nirizr self-assigned this Nov 8, 2016

nirizr force-pushed the nirizr/match_results_dialog branch 3 times, most recently from bb98f09 to 251eb05 Compare November 9, 2016 02:05

nirizr mentioned this pull request Nov 9, 2016

Add web api list view filters #104

Merged

nirizr force-pushed the nirizr/match_results_dialog branch 2 times, most recently from 8d25452 to ce701f8 Compare November 10, 2016 03:02

nirizr added 13 commits February 24, 2017 00:49

Simplify Match model to ease database insertion process

e6dee94

which proved to be quite slow

Split match results requests into three: locals, remotes, matches

a2816e8

+ performance optimizations and some debugging Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Make Modal configurable and default to false in results dialog

b7be280

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Move result download logic to action, add progress bar

f4fd1b0

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Remove remote graph display placeholder in favor of planned graph

96e66f2

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Add new serialized graph dialog displaying remote functions

df4e3b2

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Add annotation view endpoint

f06a1ce

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Show remote function in an additional graph view

391f4e6

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Jump to local code on results selection

ee2c329

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Cancel results download when user cancels

ea1f429

Also, small refactor of QueryWorker and surrounding functions Signed-off-by: Nir Izraeli <nirizr@gmail.com>

don't query unneeded db field

1793faf

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Simplify cancel and reject in MatchDialog

310690e

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

nirizr force-pushed the nirizr/match_results_dialog branch from f669679 to f0da7aa Compare February 23, 2017 22:49

nirizr added 5 commits February 24, 2017 01:09

More cleanups by reusing pbar and timer in match action

ada9b5e

Plus better exception logging Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Fix previusly uncaught minor bugs

cdaa45a

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

add locals/remotes/matches to MatchResultDialog as they come instead …

d8a0c14

…caching them in MatchAction Signed-off-by: Nir Izraeli <nirizr@gmail.com>

Fix progressbar accept on match result read

01b2da1

Signed-off-by: Nir Izraeli <nirizr@gmail.com>

nirizr force-pushed the nirizr/match_results_dialog branch from f0da7aa to 01b2da1 Compare February 23, 2017 23:09

This was referenced Feb 24, 2017

post match results dialog minor changes #194

Closed

Reduce result download time by downloading results before task is complete #197

Open

shiftre requested changes Feb 25, 2017

View reviewed changes

fix CR comments

fe558f9

* more logs and a few additional comments * validate pagination network support Signed-off-by: Nir Izraeli <nirizr@gmail.com>

shiftre merged commit 6d7db90 into master Feb 27, 2017

nirizr deleted the nirizr/match_results_dialog branch June 1, 2017 21:58

nirizr restored the nirizr/match_results_dialog branch June 1, 2017 21:58

nirizr deleted the nirizr/match_results_dialog branch June 1, 2017 21:58

nirizr mentioned this pull request Sep 29, 2017

Full pagination support in idaplugin #167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add match results dialog #17

Add match results dialog #17

nirizr commented Aug 16, 2016 •

edited

Loading

nirizr commented Aug 16, 2016

shiftre commented Sep 19, 2016 •

edited

Loading

nirizr commented Sep 19, 2016

shiftre Feb 25, 2017

nirizr Feb 25, 2017

shiftre Feb 25, 2017

nirizr Feb 25, 2017

shiftre Feb 25, 2017

nirizr Feb 25, 2017

shiftre Feb 25, 2017

nirizr Feb 25, 2017 •

edited

Loading

nirizr commented Feb 27, 2017

Add match results dialog #17

Add match results dialog #17

Conversation

nirizr commented Aug 16, 2016 • edited Loading

nirizr commented Aug 16, 2016

shiftre commented Sep 19, 2016 • edited Loading

nirizr commented Sep 19, 2016

shiftre Feb 25, 2017

Choose a reason for hiding this comment

nirizr Feb 25, 2017

Choose a reason for hiding this comment

shiftre Feb 25, 2017

Choose a reason for hiding this comment

nirizr Feb 25, 2017

Choose a reason for hiding this comment

shiftre Feb 25, 2017

Choose a reason for hiding this comment

nirizr Feb 25, 2017

Choose a reason for hiding this comment

shiftre Feb 25, 2017

Choose a reason for hiding this comment

nirizr Feb 25, 2017 • edited Loading

Choose a reason for hiding this comment

nirizr commented Feb 27, 2017

nirizr commented Aug 16, 2016 •

edited

Loading

shiftre commented Sep 19, 2016 •

edited

Loading

nirizr Feb 25, 2017 •

edited

Loading