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
=== modified file 'ibid/utils.py'
--- ibid/utils.py 2009-02-21 20:06:39 +0000
+++ ibid/utils.py 2009-02-28 21:09:37 +0000
@@ -1,5 +1,8 @@
1from htmlentitydefs import name2codepoint1from htmlentitydefs import name2codepoint
2import os.path
2import re3import re
4import time
5import urllib2
36
4def ago(delta, units=None):7def ago(delta, units=None):
5 parts = []8 parts = []
@@ -28,3 +31,31 @@
28def decode_htmlentities(string):31def decode_htmlentities(string):
29 entity_re = re.compile("&(#?)(\d{1,5}|\w{1,8});")32 entity_re = re.compile("&(#?)(\d{1,5}|\w{1,8});")
30 return entity_re.subn(substitute_entity, string)[0]33 return entity_re.subn(substitute_entity, string)[0]
34
35def cacheable_download(url, cachefile):
36 "Download url to cachefile if it's modified since cachefile"
37
38 exists = os.path.isfile(cachefile)
39
40 req = urllib2.Request(url)
41
42 if exists:
43 modified = os.path.getmtime(cachefile)
44 modified = time.strftime("%a, %d %b %Y %H:%M:%S GMT", time.gmtime(modified))
45 req.add_header("If-Modified-Since", modified)
46
47 try:
48 connection = urllib2.urlopen(req)
49 except urllib2.HTTPError, e:
50 if e.code == 304 and exists:
51 return
52 else:
53 raise
54
55 outfile = file(cachefile, "wb")
56 buf = "x"
57 while len(buf) > 0:
58 buf = connection.read(1024)
59 outfile.write(buf)
60
61 outfile.close()

Subscribers

People subscribed via source and target branches