Opened 11 years ago

Closed 11 years ago

#10301 closed enhancement (fixed)

load does not recognize https urls

Reported by: jason Owned by: jason
Priority: minor Milestone: sage-4.6.2
Component: misc Keywords: beginner
Cc: Merged in: sage-4.6.2.rc0
Authors: Ryan Grout, Alain Filbois Reviewers: Jason Grout
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

load currently only checks for remote http:// urls, not https:// urls. This should be a very simple fix in devel/sage/sage/misc/preparser.py, line 1491 or so.

Attachments (1)

trac_10301_https_urls2.patch (1.2 KB) - added by alain054 11 years ago.
Apply only this patch

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by ryan

  • Status changed from new to needs_review

anyone have an https:// url that we could use as test?

comment:2 Changed 11 years ago by ryan

  • Authors set to Ryan Grout

comment:3 Changed 11 years ago by jason

I discovered this problem when trying to load a library straight from github: https://github.com/jasongrout/minimum_rank

This worksheet loads the library and does some computations. With your patch, you should be able to change the "http" in the first cell to "https" and everything should work fine:

http://sage.cs.drake.edu/home/pub/66/

comment:4 Changed 11 years ago by ryan

added doctest

comment:5 Changed 11 years ago by jason

Oops. Any doctest that depends on the internet should be followed by "# optional - internet " like the doctest above it.

In the doctesting framework, that ensures that the doctest is not run unless you have internet available (and you specifically request the internet doctests to be run).

Changed 11 years ago by alain054

Apply only this patch

comment:6 Changed 11 years ago by alain054

  • Authors changed from Ryan Grout to Ryan Grout, Alain Filbois

This patch fixes the previous comment.

comment:7 Changed 11 years ago by jason

  • Status changed from needs_review to positive_review

Looks good! Thanks!

I verified that the doctests pass, but do not access the internet unless the right optional switches are specified.

I just had someone today get the error that this patch fixes, so I'm hoping this will go in soon.

comment:8 Changed 11 years ago by jason

I deleted the first patch since the second patch supersedes it. The only change between the two patches was the addition of an #optional - internet comment after the doctest.

comment:9 Changed 11 years ago by jason

One more note: I didn't know startswith could accept tuples. Thanks for helping me learn about Python.

comment:10 Changed 11 years ago by jdemeyer

  • Reviewers set to Jason Grout

comment:11 Changed 11 years ago by jdemeyer

  • Merged in set to sage-4.6.2.rc0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.