Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove twisted spkg #11874

Closed
dimpase opened this issue Sep 30, 2011 · 113 comments
Closed

Remove twisted spkg #11874

dimpase opened this issue Sep 30, 2011 · 113 comments

Comments

@dimpase
Copy link
Member

dimpase commented Sep 30, 2011

The version of Twisted currently used in Sage is old and obsolete; moreover it is buggy, as our extensive tests involving running labs for 120 students show. The crashes of the notebook webapp go away after the upgrade to twisted 11.

The twisted package is now included in the flask sagenb (#11080), so this ticket just removes the twisted spkg.

See http://groups.google.com/group/sage-devel/browse_thread/thread/98c53d17166da1b9 for the sage-devel vote.

apply:

  1. attachment: trac-11874-remove-twisted.patch to the sage root repository
  2. attachment: sage-spkg-11874.patch to the Sage library.

And remove the twisted spkg from the standard spkgs.

Depends on #12329
Depends on #11080
Depends on #10492

CC: @rkirov @kini @ppurka @kcrisman @jasongrout @strogdon

Component: packages: standard

Keywords: twisted, notebook

Author: Radoslav Kirov, Jason Grout

Reviewer: Dmitrii Pasechnik, Jason Grout

Merged: sage-5.2.beta0

Issue created by migration from https://trac.sagemath.org/ticket/11874

@jasongrout
Copy link
Member

comment:3

See also #11497, which has a possible fix that we may need to apply, maybe?

@jasongrout

This comment has been minimized.

@jasongrout
Copy link
Member

comment:4

See also #11497, which has a possible fix that we may need to apply, maybe?

@kcrisman
Copy link
Member

comment:5

Replying to @jasongrout:

See also #11497, which has a possible fix that we may need to apply, maybe?

Apparently not... see that ticket.

@jdemeyer
Copy link

Replying to @dimpase:

The new sage notebook at http://code.google.com/r/jasongrout-flask-sagenb/ is needed to test this package

Please explain what you mean with this: needed to "test" this package?

@jasongrout
Copy link
Member

comment:7

The changes necessary to test this package are included in the http://code.google.com/r/jasongrout-flask-sagenb/ notebook, and not in the current notebook. Thus, to test this spkg, someone will need to install the new flask server as well.

@jdemeyer
Copy link

comment:8

Replying to @jasongrout:

The changes necessary to test this package are included in the http://code.google.com/r/jasongrout-flask-sagenb/ notebook, and not in the current notebook. Thus, to test this spkg, someone will need to install the new flask server as well.

You mean also to "use" this package, or only to "test" it? That's confusing. Is flask-sagenb required or only recommended? Should this be merged into Sage or not?

Also, I would really appreciate some kind of roadmap from the flask-sagenb people to eventually be merged into Sage (I've said this many times without any good answer). Because it looks more and more that flask-sagenb is a fork of sagenb with no real plan for eventually getting back into Sage.

@jasongrout
Copy link
Member

comment:9

You need the flask-sagenb to use or test this spkg (unless someone backports the necessary patches to the current sagenb, but I don't see that happening).

Sorry for the confusion about the flask-sagenb. Part of the reason is that I've been concentrating so much on fixing bugs with the limited time I had.

That bug-fixing work is winding down as we see fewer and fewer errors popping up on sagenb.org. Over the next few weeks, I'm turning my attention to packaging flask-sagenb up, with the dependencies, and submitting it to trac. The goal is to work on this at SD 35.5, if the packaging work isn't already done by then. Basically, I need to package up these packages: http://code.google.com/p/sagenb/issues/detail?id=62 and also package up the actual sagenb as well.

@jasongrout
Copy link
Member

comment:10

The start of that packaging process is this ticket. Since I saw we already had a ticket about upgrading twisted, I put our spkg we already have been testing on *.sagenb.org up here.

@jasongrout
Copy link
Member

comment:11

To be more clear, here are the priority things to finish before releasing sagenb-flask officially:

http://code.google.com/p/sagenb/issues/list?can=2&q=label:flask%20status:Accepted&sort=priority&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary

(the low priority items could be bumped to a later release)

@kcrisman
Copy link
Member

comment:12

Replying to @jasongrout:

To be more clear, here are the priority things to finish before releasing sagenb-flask officially:

http://code.google.com/p/sagenb/issues/list?can=2&q=label:flask%20status:Accepted&sort=priority&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary

I would say that [Safari not saving correctly http://code.google.com/p/sagenb/issues/detail?id=60] is a pretty important thing, though apparently it's not universal for some reason. (This still happens, as far as I can tell.)

@jasongrout
Copy link
Member

comment:13

Yes, but that's a Safari bug, as far as we can tell, and it's hard to reproduce. Is it a regression from the old notebook? If it's not a regression, it may not make it into the first sagenb-flask release.

That said, did you test it again with sagenb.org? I see that Safari has yet another update, 5.1.2, which looks like it may have fixed the underlying bug.

I sent a message to sage-notebook about these updates; we should continue our conversation there.

@kcrisman
Copy link
Member

comment:14

Replying to @jasongrout:

Yes, but that's a Safari bug, as far as we can tell, and it's hard to reproduce. Is it a regression from the old notebook? If it's not a regression, it may not make it into the first sagenb-flask release.

I thought it was a regression, otherwise I wouldn't have mentioned it. But upon reading my own original post about this, apparently it's a regression unrelated to flask. Sorry for the noise w.r.t. this ticket.

That said, did you test it again with sagenb.org? I see that Safari has yet another update, 5.1.2, which looks like it may have fixed the underlying bug.

I'll try that as well as your other comment on googlecode.

I sent a message to sage-notebook about these updates; we should continue our conversation there.

Ok.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:16

Replying to @jasongrout:

That bug-fixing work is winding down as we see fewer and fewer errors popping up on sagenb.org. Over the next few weeks, I'm turning my attention to packaging flask-sagenb up, with the dependencies, and submitting it to trac. The goal is to work on this at SD 35.5, if the packaging work isn't already done by then. Basically, I need to package up these packages: http://code.google.com/p/sagenb/issues/detail?id=62 and also package up the actual sagenb as well.

Thanks for the update and apologies if I sounded a bit harsh and inpolite in my comment.

@dimpase
Copy link
Member Author

dimpase commented Dec 27, 2011

comment:18

I would like to give this a positive review.

@simon-king-jena
Copy link
Member

comment:93

Apparently the instructions at #11080 where indeed misleading: It is not stated that one should start with sage-5.0.beta13. sage-5.0.prealpha0 does not suffice.

@kini
Copy link
Contributor

kini commented May 2, 2012

comment:94

So Simon, does everything seem to work with a recent beta (or rc0)?

@gutow
Copy link

gutow commented May 28, 2012

comment:95

As #11080 has a positive review, shouldn't this one also?

@dimpase
Copy link
Member Author

dimpase commented May 29, 2012

comment:96

everything works on Sage 5.0.

@jdemeyer
Copy link

jdemeyer commented Jul 2, 2012

Merged: sage-5.2.beta0

@jdemeyer
Copy link

Changed merged from sage-5.2.beta0 to none

@jdemeyer
Copy link

comment:99

Unmerging this from sage-5.2 due to the serious security issue at #13270.

@jdemeyer jdemeyer removed this from the sage-5.2 milestone Jul 23, 2012
@jdemeyer jdemeyer reopened this Jul 23, 2012
@jdemeyer
Copy link

Merged: sage-5.2.beta0

@jdemeyer jdemeyer added this to the sage-5.2 milestone Jul 25, 2012
@jdemeyer jdemeyer removed the pending label Jul 25, 2012
@fchapoton
Copy link
Contributor

Changed author from Rado Kirov, Jason Grout to Radoslav Kirov, Jason Grout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests