Ticket #5712 (positive_review defect)

Opened 12 months ago

Last modified 9 days ago

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-5.0
Component: notebook Keywords:
Cc: acleone, was, boothby, mpatel Author(s): Tim Dumol, Mitesh Patel
Report Upstream: N/A Reviewer(s): Mitesh Patel, Tim Dumol, Alex Leone
Merged in: Work issues:

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

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

Change History

  Changed 12 months 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.

  Changed 11 months 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 2 months ago by timdumol

Uses Achtung (jQuery library) for notifications instead.

  Changed 2 months ago by timdumol

  • cc was added
  • status changed from new to needs_review
  • upstream set to N/A
  • author set to Tim Dumol

This should do the thing.

Changed 2 months ago by timdumol

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

  Changed 2 months ago by timdumol

  • cc boothby added

  Changed 2 months 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

  Changed 2 months ago by timdumol

  • cc mpatel added

follow-up: ↓ 8   Changed 2 months 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."?

in reply to: ↑ 7 ; follow-up: ↓ 9   Changed 2 months ago by timdumol

  • reviewer 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."?

in reply to: ↑ 8   Changed 2 months 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.

  Changed 2 months ago by timdumol

This patch should do it.

Changed 2 months ago by timdumol

Adds the requested changes.

Changed 2 months ago by mpatel

See comment. Replaces previous.

Changed 2 months ago by mpatel

Periodically re-attempt to interrupt. Replaces previous.

  Changed 2 months 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 8 weeks ago by mpatel

Rebased against SageNB 0.7 at #8051. Replaces previous.

  Changed 8 weeks ago by mpatel

V6 is rebased against #8051's SageNB 0.7.

  Changed 8 weeks ago by mpatel

  • cc acleone added

  Changed 9 days ago by timdumol

  • reviewer changed from Mitesh Patel to Mitesh Patel, Tim Dumol
  • author changed from Tim Dumol to Tim Dumol, Mitesh Patel

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?

  Changed 9 days ago by acleone

  • status changed from needs_review to positive_review
  • reviewer changed from Mitesh Patel, Tim Dumol to Mitesh Patel, Tim Dumol, Alex Leone

LGTM.

Note: See TracTickets for help on using tickets.