Issue 2144 - [adblockplus.org Anwiki to CMS migration] get_subscriptions filter expects a local sub…

Issue 2144 - [adblockplus.org Anwiki to CMS migration] get_subscriptions filter expects a local sub…

[Sebastian Noack, 2015-03-13 19:51:44]

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
filters/get_subscriptions.py:23: import
sitescripts.subscriptions.subscriptionParser as subscriptionParser
Nit: from ... import

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
filters/get_subscriptions.py:32:
settings.readfp(utf8_reader(urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")))
The file-like object returned by urllib.urlopen() wants to be closed. Consider a
with-block.

[Wladimir Palant, 2015-03-13 19:58:51]

LGTM

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
filters/get_subscriptions.py:23: import
sitescripts.subscriptions.subscriptionParser as subscriptionParser
On 2015/03/13 19:51:45, Sebastian Noack wrote:
> Nit: from ... import

Done.

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
filters/get_subscriptions.py:32:
settings.readfp(utf8_reader(urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")))
On 2015/03/13 19:51:45, Sebastian Noack wrote:
> The file-like object returned by urllib.urlopen() wants to be closed. Consider
a
> with-block.

Done.

[Sebastian Noack, 2015-03-13 20:06:34]

LGTM

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5629499534213120/filt...
filters/get_subscriptions.py:32:
settings.readfp(utf8_reader(urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")))
On 2015/03/13 19:58:51, Wladimir Palant wrote:
> On 2015/03/13 19:51:45, Sebastian Noack wrote:
> > The file-like object returned by urllib.urlopen() wants to be closed.
Consider
> a
> > with-block.
> 
> Done.

I verified that __enter__/__exit__ isnt't implemente here, hence a try/finally
block is needed.

http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
filters/get_subscriptions.py:32: settings_handle =
urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")
Consider moving that logic into subscriptionParser where we could keep using hg,
just falling back to a remote URL, if there is no local repo, or something like
that.

[Wladimir Palant, 2015-03-13 20:08:54]

http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
File filters/get_subscriptions.py (right):

http://codereview.adblockplus.org/5662732741246976/diff/5724160613416960/filt...
filters/get_subscriptions.py:32: settings_handle =
urllib.urlopen("https://hg.adblockplus.org/subscriptionlist/rawfile/default/settings")
On 2015/03/13 20:06:34, Sebastian Noack wrote:
> Consider moving that logic into subscriptionParser where we could keep using
hg,
> just falling back to a remote URL, if there is no local repo, or something
like
> that.

Yes, that's the idea - the hacks here are merely the short-term solution.