Ibid

Merge lp:~stefanor/ibid/rfc-plugin-330864 into lp:~ibid-core/ibid/old-trunk-pack-0.92

  1. rfc-plugin-330864
  2. Merge into old-trunk-pack-0.92
Proposed by Stefano Rivera
Status: Merged
Approved by: Jonathan Hitchcock
Approved revision: 562
Merged at revision: 558
Proposed branch: lp:~stefanor/ibid/rfc-plugin-330864
Merge into: lp:~ibid-core/ibid/old-trunk-pack-0.92
Diff against target: None lines
To merge this branch: bzr merge lp:~stefanor/ibid/rfc-plugin-330864
Reviewer Review Type Date Requested Status
Jonathan Hitchcock Approve
Michael Gorven Approve
Review via email: mp+4040@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Stefano Rivera (stefanor) wrote :

I'm sure I'll get complaints with my approach on this one, but one has to start the discussion somewhere :-)

* Thoughts on the caching interface? Surely the cachedir should be specified globally, but the API doesn't have any convenient hooks for that that I can see.
  - In that case, plugin cached-files should probably go in /cachedir/plugin/file.foo

* The search engine ain't so great. Anyone want to write something awesome?

lp:~stefanor/ibid/rfc-plugin-330864 updated
552. By Stefano Rivera

A simple RFC parser

Revision history for this message
Jonathan Hitchcock (vhata) wrote :

10:30 -+- Vhata reads cacheable_download()
10:30 <Vhata> iiiiinteresting
10:31 <Vhata> what do you think about cacheable_download(url, cachefilename), and then cachefile = "%s/%s" % (config['cachedir'], cachefilename), after validating cachefilename to exclude '/' ?
10:35 <tumbleweed> Vhata: firstly, os.path.join
10:35 <tumbleweed> Vhata: suits me, although I'm tempted to do cachedir/plugin/filename
10:36 <Vhata> sorry, you're right :)
10:36 <Vhata> um
10:36 <Vhata> multiple plugins might want the same file
10:36 <Vhata> buuuut
10:36 <Vhata> multiple plugins might want different files with the same name
10:36 <Vhata> buuuuut
10:37 <Vhata> how do you know which plugin is calling it?
10:38 <tumbleweed> Vhata: good point
10:38 <tumbleweed> how about having that as a standard guideline
10:38 <tumbleweed> so if you write a plugin, you should request pluginname/file
10:39 <tumbleweed> that'll also allow you to poke in other people's files if you know what you are doing
10:43 <Vhata> plugins should kinda be independent
10:43 <Vhata> which invalidates my first point
10:43 <Vhata> so, ja, request pluginname/file
10:44 <Vhata> and validating cachefilename isn't really necessary, because plugins aren't supposed to be security risks, surely
10:46 <tumbleweed> my thoughts too
10:46 <tumbleweed> although detecting ".." and "~" would probably do the trick

review: Needs Fixing
lp:~stefanor/ibid/rfc-plugin-330864 updated
553. By Stefano Rivera

Merge from trunk

554. By Stefano Rivera

Overhaul cacheable_download to be more suitable for other plugins.
Improve error handling.
Make RFC index URL configurable.

Revision history for this message
Stefano Rivera (stefanor) wrote :

Rev 555 fixes all of your issues

lp:~stefanor/ibid/rfc-plugin-330864 updated
555. By Stefano Rivera

Unicode cleanups.
Support Regular Expression searches.
Increase postel-level.

556. By Stefano Rivera

OSError isn't in os

Revision history for this message
Michael Gorven (mgorven) wrote :

* Regex search should be case insensitive.
* cacheable_download() should include a default cachedir.
* I would prefer search results to include less information. The authors and
things like "(Format: TXT=43493 bytes)" aren't really useful. Possibly
something like:
5336: "SMTP Extension for Internationalized Email Addresses", September 2008,
EXPERIMENTAL

 review needs_fixing

review: Needs Fixing
lp:~stefanor/ibid/rfc-plugin-330864 updated
557. By Stefano Rivera

Search regex should be case insensitive

558. By Stefano Rivera

Provide a default cache dir

559. By Stefano Rivera

Show summaries in rfc search.

(Includes one god-awful regex that nobody in their right mind would merge)

Revision history for this message
Stefano Rivera (stefanor) wrote :

"I would prefer search results to include less information."

You *really* shouldn't have said that. Current best-effort code.

lp:~stefanor/ibid/rfc-plugin-330864 updated
560. By Stefano Rivera

Lazy parsing - much faster

Revision history for this message
Michael Gorven (mgorven) wrote :

Exception when downloading RFC index:
  File "/home/mgorven/lp/stefanor/ibid/rfc-plugin-330864/ibid/utils.py", line
86, in cacheable_download
    os.path.rename(downloadfile, cachefile)
AttributeError: 'module' object has no attribute 'rename'

Search results should preferably be returned on a single line. This may
require removing some information like the date.

lp:~stefanor/ibid/rfc-plugin-330864 updated
561. By Stefano Rivera

Was calling functions in the wrong OS module

562. By Stefano Rivera

Single line responses.

Revision history for this message
Michael Gorven (mgorven) wrote :

Looks good now.
 review approve

review: Approve
Revision history for this message
Jonathan Hitchcock (vhata) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ibid/utils.py'
2--- ibid/utils.py 2009-02-21 20:06:39 +0000
3+++ ibid/utils.py 2009-02-28 21:09:37 +0000
4@@ -1,5 +1,8 @@
5 from htmlentitydefs import name2codepoint
6+import os.path
7 import re
8+import time
9+import urllib2
10
11 def ago(delta, units=None):
12 parts = []
13@@ -28,3 +31,31 @@
14 def decode_htmlentities(string):
15 entity_re = re.compile("&(#?)(\d{1,5}|\w{1,8});")
16 return entity_re.subn(substitute_entity, string)[0]
17+
18+def cacheable_download(url, cachefile):
19+ "Download url to cachefile if it's modified since cachefile"
20+
21+ exists = os.path.isfile(cachefile)
22+
23+ req = urllib2.Request(url)
24+
25+ if exists:
26+ modified = os.path.getmtime(cachefile)
27+ modified = time.strftime("%a, %d %b %Y %H:%M:%S GMT", time.gmtime(modified))
28+ req.add_header("If-Modified-Since", modified)
29+
30+ try:
31+ connection = urllib2.urlopen(req)
32+ except urllib2.HTTPError, e:
33+ if e.code == 304 and exists:
34+ return
35+ else:
36+ raise
37+
38+ outfile = file(cachefile, "wb")
39+ buf = "x"
40+ while len(buf) > 0:
41+ buf = connection.read(1024)
42+ outfile.write(buf)
43+
44+ outfile.close()

Subscribers

People subscribed via source and target branches