Opened 10 years ago

Closed 9 years ago

#5712 closed defect (fixed)

notebook -- Get rid of the stupid "unable to immediately interrupt computation" alert that drives me nuts

Reported by: was Owned by: boothby
Priority: major Milestone: sage-4.4.2
Component: notebook Keywords:
Cc: acleone, was, boothby, mpatel Merged in: sagenb-0.8
Authors: Tim Dumol, Mitesh Patel Reviewers: Mitesh Patel, Tim Dumol, Alex Leone
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

More precisely, one could keep the alert, but make it so that it can *not* appear if it already appeared in the last 5 seconds (say). It is so annoying!

Attachments (6)

trac_5712-interrupt-notification.patch (32.3 KB) - added by timdumol 9 years ago.
Uses Achtung (jQuery library) for notifications instead.
trac_5712-interrupt-notification.2.patch (35.9 KB) - added by timdumol 9 years ago.
Fixes #7702 and a typo in notebook_lib.js from the previous patch. Apply this patch alone.
trac_5712-interrupt-notification.3.patch (36.6 KB) - added by timdumol 9 years ago.
Adds the requested changes.
trac_5712-interrupt-notification.4.patch (36.6 KB) - added by mpatel 9 years ago.
See comment. Replaces previous.
trac_5712-interrupt-notification.5.patch (89.0 KB) - added by mpatel 9 years ago.
Periodically re-attempt to interrupt. Replaces previous.
trac_5712-interrupt-notification.6.patch (88.9 KB) - added by mpatel 9 years ago.
Rebased against SageNB 0.7 at #8051. Replaces previous.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 10 years ago by boothby

The "2.0" thing to do is to inject a div somewhere, and just keep sending interrupts to the server until it works or the interrupt process gets cancelled.

"Couldn't immediately interrupt. Trying again in (seconds 'till next retry) [cancel] [restart worksheet]"

[x] <- is a button that says x.

comment:2 Changed 10 years ago by mabshoff

  • Milestone changed from sage-3.4.1 to sage-3.4.2

Is this a high priority issue? I am not convinced and since there is no patch and no sign of anyone working on fixing this I am bumping this to 3.4.2.

Cheers,

Michael

Changed 9 years ago by timdumol

Uses Achtung (jQuery library) for notifications instead.

comment:3 Changed 9 years ago by timdumol

  • Authors set to Tim Dumol
  • Cc was added
  • Report Upstream set to N/A
  • Status changed from new to needs_review

This should do the thing.

Changed 9 years ago by timdumol

Fixes #7702 and a typo in notebook_lib.js from the previous patch. Apply this patch alone.

comment:4 Changed 9 years ago by timdumol

  • Cc boothby added

comment:5 Changed 9 years ago by timdumol

To note, the patch series for this is:

trac_7650-sagenb_doctesting_v6.patch
trac_7650-reviewer.patch
trac_7648-missing_indent.patch
trac_7663-rstrip_prompt.patch
trac_7847-empty-trash-no-referer.patch
trac_7786-template-jinja-idiomatic.patch
trac_7863-declare_vars_aux_js_v2.patch
trac_7874-typeset_interact_labels.patch
trac_7858-key_binding_vars_v2.patch
trac_7666-alphanumeric_cell_ids_B5.patch
trac_7666-reviewer.patch
trac_7835-preparsing-unicode_v2.patch
trac_7249_jinja2_v5.patch
trac_2779-sagenb-error-message.patch
2779_2_banner.patch
trac_3154-spurious-u0027-output.patch
trac_7969-escaped-backslash.patch
trac_7937-sass_manifest.patch
trac_4217-html-system-formatting.patch
trac_3083-print-documentation.patch
trac_7962-link-worksheets-zip-file.patch
trac_5177-delete-cell-dirs.patch
trac_5712-interrupt-notification.patch

comment:6 Changed 9 years ago by timdumol

  • Cc mpatel added

comment:7 follow-up: Changed 9 years ago by mpatel

  • I get a doctest failure from Worksheet.interrupt.
  • Just to check: Is _ui-achtung.sass just ui.achtung.css run through css2sass? I suggest describing the conversion method briefly in sass/readme.txt.
  • Interrupting an idle worksheet also brings up the alert.
  • V2 does not fix #7702's example (interrupt test.sws). A new alert appears every time I try to interrupt the calculation, but the calculation keeps going (e.g., according to top). (I stopped after about 10 attempts.) Restarting the worksheet does stop the calculation. But this need not hold up this ticket, as I can interrupt other examples, such as import time; time.sleep(10) and factor(factorial(100000000)).
  • When we do show the alert (with #7702's example or a different one), maybe we should add, e.g., "Please try again in a few seconds. If that doesn't work, please try restarting the worksheet."?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 9 years ago by timdumol

  • Reviewers set to Mitesh Patel

Replying to mpatel:

  • I get a doctest failure from Worksheet.interrupt.
  • Just to check: Is _ui-achtung.sass just ui.achtung.css run through css2sass? I suggest describing the conversion method briefly in sass/readme.txt.

Yes. Sure, I'll do so.

  • Interrupting an idle worksheet also brings up the alert.

Good catch.

  • V2 does not fix #7702's example (interrupt test.sws). A new alert appears every time I try to interrupt the calculation, but the calculation keeps going (e.g., according to top). (I stopped after about 10 attempts.) Restarting the worksheet does stop the calculation. But this need not hold up this ticket, as I can interrupt other examples, such as import time; time.sleep(10) and factor(factorial(100000000)).

The point of #7702 is not to interrupt uninterruptible calculations, but to recognize that a calculation cannot be interrupted and display an appropriate alert.

  • When we do show the alert (with #7702's example or a different one), maybe we should add, e.g., "Please try again in a few seconds. If that doesn't work, please try restarting the worksheet."?

comment:9 in reply to: ↑ 8 Changed 9 years ago by timdumol

Replying to timdumol:

Replying to mpatel:

  • I get a doctest failure from Worksheet.interrupt.
  • Just to check: Is _ui-achtung.sass just ui.achtung.css run through css2sass? I suggest describing the conversion method briefly in sass/readme.txt.

Yes. Sure, I'll do so.

In another ticket, that is.

  • Interrupting an idle worksheet also brings up the alert.

Good catch.

  • V2 does not fix #7702's example (interrupt test.sws). A new alert appears every time I try to interrupt the calculation, but the calculation keeps going (e.g., according to top). (I stopped after about 10 attempts.) Restarting the worksheet does stop the calculation. But this need not hold up this ticket, as I can interrupt other examples, such as import time; time.sleep(10) and factor(factorial(100000000)).

The point of #7702 is not to interrupt uninterruptible calculations, but to recognize that a calculation cannot be interrupted and display an appropriate alert.

  • When we do show the alert (with #7702's example or a different one), maybe we should add, e.g., "Please try again in a few seconds. If that doesn't work, please try restarting the worksheet."?

Will do so.

comment:10 Changed 9 years ago by timdumol

This patch should do it.

Changed 9 years ago by timdumol

Adds the requested changes.

Changed 9 years ago by mpatel

See comment. Replaces previous.

Changed 9 years ago by mpatel

Periodically re-attempt to interrupt. Replaces previous.

comment:11 Changed 9 years ago by mpatel

Please ignore V4. V5

  • Uses the new callbacks to send additional interrupt requests and update an alert every 5 seconds, until a request succeeds or the user closes the alert. If the process is interrupted, or the user restarts it, or the computation is done, etc., the alert should close itself.

Please test (and post feedback)!

Changed 9 years ago by mpatel

Rebased against SageNB 0.7 at #8051. Replaces previous.

comment:12 Changed 9 years ago by mpatel

V6 is rebased against #8051's SageNB 0.7.

comment:13 Changed 9 years ago by mpatel

  • Cc acleone added

comment:14 Changed 9 years ago by timdumol

  • Authors changed from Tim Dumol to Tim Dumol, Mitesh Patel
  • Reviewers changed from Mitesh Patel to Mitesh Patel, Tim Dumol

Works for me against 0.7.5.3. Can't see anything wrong with the changes. Positive review for your changes. Should we mark this positive review, or ask for a third party?

comment:15 Changed 9 years ago by acleone

  • Reviewers changed from Mitesh Patel, Tim Dumol to Mitesh Patel, Tim Dumol, Alex Leone
  • Status changed from needs_review to positive_review

LGTM.

comment:16 Changed 9 years ago by timdumol

  • Merged in set to sagenb-0.8

comment:17 Changed 9 years ago by timdumol

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.