Don't hardcode retrieval of subscriptionlist content in get_subscriptions()

[greiner]

Background

Testing subscriptionlist changes on adblockplus.org/subscriptions requires changes to web.adblockplus.org. More specifically, the subscriptionlist repository content is fetched from the remote source. So to be able to refer to a local source it requires changes to globals/get_subscriptions.py which includes hardcoded URLs to the source.

What to change

1. Take the URL of the repository from settings.ini of the website. The key should be called subscriptions_repo. The default value of the URL should be https://hg.adblockplus.org/subscriptionlist (the repo that is used now).
2. In case the subscriptions repo is remote, use the strategy that is used now (fetch settings file individually and get the rest as a .tgz archive).
3. In case the subscriptions repo is local, read the files from the currently checked out working copy. Any non-committed changes would also be included.
4. If the environment variable CMS_SUBSCRIPTIONS_REPO is set, it should override the value from settings.ini (or the default). This would allow pointing the script to a local subscriptions repository without changing any files in the website repository.

[greiner]

This code is part of the web.adblockplus.org repository (see ​globals/get_subscriptions.py). Apparently, it was moved to a different directory though so I updated the path that's mentioned in the issue description to reflect that.

[juliandoucette]

This looks doable.

@kvas can you please confirm the priority of this issue (I think it's P4) and feel free to tackle it. Otherwise, I fear it will be here a while.

[juliandoucette]

(Sorry @kzar, I meant @kvas.)

[kzar]

Sure, no problem.

[kvas]

Two questions:

• I'm not sure why you want to use sitescripts.ini here. It seems that this configuration variable has nothing to do with sitescripts (or am I wrong?). Would not using settings.ini of the website make more sense?
• Do you think that the default behavior (if no repository is specified in the configuration) should stay the way it is now?

[greiner]

Replying to kvas:

• I'm not sure why you want to use sitescripts.ini here. It seems that this configuration variable has nothing to do with sitescripts (or am I wrong?). Would not using settings.ini of the website make more sense?

At the time when the ticket was created the get_subscriptions() function was still part of the sitescripts repo which is why I suggested "sitescripts.ini" (possibly also to avoid duplication). From what I see it is now located in the web.adblockplus.org repo, however, so feel free to modify the ticket description accordingly.

• Do you think that the default behavior (if no repository is specified in the configuration) should stay the way it is now?

It'd be helpful to get some warning or error to clarify what needs to be changed to get it to work but yeah, generally I don't expect the default behavior to change.

[kvas]

It seems that the best way forward for now would be to change the logic of this script to clone the subscriptions repository and then work with the local clone instead of streaming the .tgz archive from the server and extracting the files on the fly. Then for testing we could just point the macro to a local copy using an environment variable or a key in the website settings.ini. I will check with the original author of the macro if this doesn't break any useful existing behavior and then the ticket would be ready to work.

[kvas]

Having thought about it again, I would say that it seems better to leave the default behavior as is (streaming .tgz file from server and unpacking it on the fly). It produces less network traffic and we don't need to worry about where to put the clone of the repo. The only thing to change here is that the url of the archive should be taken from settings.ini of the website.

Then, as we agreed, if the url is a local directory, we just read the subscriptions from there instead. And we can also add an environment variable for overriding the subscriptions url -- this would make debugging even easier.

If this seems fine, I can update the description of the issue and we can set it to Ready and give it a proper priority.

[juliandoucette]

If this seems fine, I can update the description of the issue and we can set it to Ready and give it a proper priority.

Seems fine to me.

[juliandoucette]

I'm going to give this to you instead of migrating it to gitlab kvas. If you disagree please create an issue on gitlab or ping me on IRC.

[juliandoucette]

Migrating this instead as discussed on IRC.

[juliandoucette]

Migrated to ​https://gitlab.com/eyeo/websites/web.adblockplus.org/issues/52