Wallpapers.net doesn't work as source of wallpapers

Bug #1628074 reported by Alberto Salvia Novella
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Variety
Fix Released
Undecided
Unassigned
variety (Ubuntu)
Fix Released
Low
Unassigned
Xenial
Fix Committed
Low
Unassigned

Bug Description

The listed source of wallpapers (http://wallpapers.net/nature-desktop-wallpapers.html) doesn't download wallpapers. Correcting the link doesn't work either.

ProblemType: Bug
DistroRelease: Ubuntu 16.04
Package: variety 0.6.0-1
ProcVersionSignature: Ubuntu 4.4.0-38.57-generic 4.4.19
Uname: Linux 4.4.0-38-generic x86_64
ApportVersion: 2.20.1-0ubuntu2.1
Architecture: amd64
CurrentDesktop: Unity
Date: Tue Sep 27 14:07:06 2016
InstallationDate: Installed on 2016-05-02 (147 days ago)
InstallationMedia: Ubuntu 16.04 LTS "Xenial Xerus" - Release amd64 (20160420.1)
PackageArchitecture: all
SourcePackage: variety
UpgradeStatus: No upgrade log present (probably fresh install)

----
Below is the SRU template (James Lu):

[Impact]

Variety support for Wallpapers.net no longer works because the site has since changed its layout.
Newer versions of Variety have dropped support for this source, and the SRU patch below backports this removal in Xenial.

Per the bug nomination for Xenial and Section 2.1 of https://wiki.ubuntu.com/StableReleaseUpdates#Other_safe_cases (A library for a web service needs to be updated for changes to the web server API), I believe this fix is worthwhile in improving user experience and unlikely to cause serious regressions.

 * An explanation of the effects of the bug on users and

 * justification for backporting the fix to the stable release.

 * In addition, it is helpful, but not required, to include an
   explanation of how the upload fixes this bug.

[Test Case]

* In the Images list of Variety's preferences page, attempting to select Wallpapers.net ("wn") as a wallpaper source causes a silent failure as no wallpapers are found.

* Attempting to add any Wallpapers.net category (e.g. http://www.wallpapers.net/wallpapers/nature) using the Preferences->General->Add button fails with a "Could not find wallpapers there" error.

[Regression Potential]

From my testing, when this patch is applied, all previous sources using Wallpapers.net are removed (this actually raises a "Unknown source" error in the console, which leads to automatic removal). This makes it possible to have a variety configuration with no wallpaper sources left after an upgrade, but this does not appear to be fatal and can be quickly fixed through the preferences page.

Some more regression potential:
- This patch removes a substantial amount of code, so it's possible that there are there are modules with lingering dependencies on the removed Wallpapers.net code. Should this be true, that code will fail to import and potentially cause Variety to crash on start.
- Part of the upstream patch removes python-lxml as a dependency, which I've done in my packaging as well. As far as I can tell (from the results of "grep lxml **/*.py"), variety/WallpapersNetDownloader.py was the only module in Variety to use lxml, and this is removed by the patch. However, if this assumption is wrong, a missing module will cause Variety to fail to start.
- It may be worth checking how other features of Variety (e.g. filters) are affected by the potential removal of all Wallpaper sources (though this can be manually done by just deselecting everything in Preferences). One note is that sync is currently broken regardless of this commit due to https://bugs.launchpad.net/variety/+bug/1635912, which is yet to be fixed upstream.

[Other Info]

The patch I'm submitting for this bug is slightly modified from the upstream commit, and I've removed conflicting edits to the package description (debian/control) and the autogenerated po/variety.pot file.

Revision history for this message
Alberto Salvia Novella (es20490446e) wrote :
Changed in variety (Ubuntu):
status: New → Triaged
Changed in variety:
status: New → Confirmed
Revision history for this message
Peter Levi (peterlevi) wrote :

Support for Wallpapers.net was officially dropped in 0.6.1 and the related UI items were removed (upstream bugreport is https://bugs.launchpad.net/variety/+bug/1588861)

Changed in variety:
status: Confirmed → Fix Released
milestone: none → 0.6.1
Changed in hundredpapercuts:
status: New → Triaged
importance: Undecided → Low
Revision history for this message
James Lu (jlu5) wrote :

In order to fix this, would it be worthwhile to backport https://bazaar.launchpad.net/~peterlevi/variety/trunk/revision/558 to Xenial as a SRU?

Revision history for this message
Peter Levi (peterlevi) wrote :

It makes sense to backport the latest trunk revision, not an intermediate one. I think the wallpapers.net bug alone is a very minor nuisance, but getting the Safe mode feature backported is quite worth it - lots of people are using that one, and also providing feedback on image safety.

Revision history for this message
James Lu (jlu5) wrote :

Stable update patches usually have to be minimal and only address important issues (this counts per section 2.1 I guess): https://wiki.ubuntu.com/StableReleaseUpdates

So unfortunately, introducing a new version is probably not possible.

Revision history for this message
James Lu (jlu5) wrote :

By backport I meant taking the commit I linked as a patch, and adding it to 0.6.0-1

Revision history for this message
Alberto Salvia Novella (es20490446e) wrote :
Revision history for this message
Amr Ibrahim (amribrahim1987) wrote :

New versions as micro-releases (bug-fix releases) can be SRUed.

Revision history for this message
James Lu (jlu5) wrote :

Versions newer than 0.6.1 have been uploaded to Ubuntu already, so this should be fixed.

Changed in variety (Ubuntu):
status: Triaged → Fix Released
James Lu (jlu5)
description: updated
Revision history for this message
Gianfranco Costamagna (costamagnagianfranco) wrote :
Changed in variety (Ubuntu Xenial):
status: New → In Progress
Revision history for this message
Brian Murray (brian-murray) wrote :

I've only skimmed the patch but it seems to me like it'd be possible to keep WallpapersNetDownloader.py and still remove the listing for wallpapers.net thereby reducing the regression potential. Is that true?

Changed in variety (Ubuntu Xenial):
status: In Progress → Incomplete
Revision history for this message
Alberto Salvia Novella (es20490446e) wrote :

@ Brian Murray

But why keeping a module which sole purpose is never invoked?

Changed in variety (Ubuntu Xenial):
importance: Undecided → Low
Revision history for this message
James Lu (jlu5) wrote :

I agree and disagree with only patching out the UI elements: while it's possible to prevent adding new Wallpapers.net sources by removing it from the GUI, doing so doesn't remove broken sources that may already exist in a user's config. New code may have to be introduced instead to properly migrate away from dead sources, while removal due to sources no longer present in Variety's supported list are automatically removed.

I will admit that I'm not sure whether auto-removal of missing sources is intentional or a side-effect of how Variety's downloader system works - it relies on exception handling, though that isn't exactly discouraged in Python. :) Maybe Peter can comment on this.

Revision history for this message
Peter Levi (peterlevi) wrote :

I'm not 100% sure this specific bug is worth patching. Various sources have stayed temporarily broken before due to website changes, and most users are kind of accustomed to this sort of breakage. This is actually one of the reasons I want to drop any non-API sources from Variety - they are a PITA to support and maintain.

But if you feel this is worth releasing a patch, I think it is better to "operate" the source completely rather than just hiding it from the UI. The change is very simple, and fairly risk-free from introducing regressions. The mentioned trunk commit http://bazaar.launchpad.net/~peterlevi/variety/trunk/revision/558 contains exactly the necessary changes, nothing less, nothing more. Only variety.pot file needs to be regenerated afterwards.
Btw, this commit gives a good "recipe" for which places in the code need touching when removing or adding a new source in the current non-pluginnable state of affairs.

Regarding auto-removal of dead sources from the config:
It is an intentional side-effect :) Basically Variety only loads from the config sources whose types it recognises and shows them in the preferences. So sources with type "wn" will not be loaded at all once support for it is removed. On the other hand, once preferences are persisted (and this happens automatically in various places), the config file is simply dumped from the currently loaded settings, so if a source wasn't there, it is removed from the config file as well. Hence - clean config with only the supported sources remaining there.

Changed in variety (Ubuntu Xenial):
status: Incomplete → Triaged
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Alberto, or anyone else affected,

Accepted variety into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/variety/0.6.0-1ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in variety (Ubuntu Xenial):
status: Triaged → Fix Committed
tags: added: verification-needed
Revision history for this message
Brian Murray (brian-murray) wrote :

I've gone ahead and accepted this SRU but given Peter's comment re "most users are kind of accustomed to this sort of breakage" wonder if it is worth doing in the future.

Revision history for this message
James Lu (jlu5) wrote :

I agree that maintaining website scraping is difficult and that constantly backporting changes to older releases isn't ideal. So, I am looking forward to a new release where difficult to maintain sources are dropped once and for all. Thanks for all the feedback everyone :)

Revision history for this message
Alberto Salvia Novella (es20490446e) wrote :

If this is relevant to you, I only use Unsplash as image source. That way the theme is always neutral, and work safe.

Revision history for this message
Łukasz Zemczak (sil2100) wrote : Change of SRU verification policy

As part of a recent change in the Stable Release Update verification policy we would like to inform that for a bug to be considered verified for a given release a verification-done-$RELEASE tag needs to be added to the bug where $RELEASE is the name of the series the package that was tested (e.g. verification-done-xenial). Please note that the global 'verification-done' tag can no longer be used for this purpose.

Thank you!

Revision history for this message
Amr Ibrahim (amribrahim1987) wrote :

Please verify the fix for Xenial.

Revision history for this message
Alberto Salvia Novella (es20490446e) wrote :

I'm sorry but right now I'm not contributing to Ubuntu. If you needed more explanation about that just click on my name in Launchpad.

As a side note, after been investigating for a while, I would mention that applications are notably more broken in Ubuntu and Debian than in Arch based distributions.

Having SRUs, scheduled releases, intermediate software buffers, and long submission processes is being the quality killer here.

Those are like building a wall for containing a river's water, with an ever increasing water level. Eventually the level overflows the wall, no matter how tall it is.

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote : [variety/xenial] verification still needed

The fix for this bug has been awaiting testing feedback in the -proposed repository for xenial for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

tags: added: removal-candidate
Revision history for this message
Paul White (paulw2u) wrote :

Further to comment #22 I'm removing the '100 Papercuts' task as 'xenial' is no longer supported.

no longer affects: hundredpapercuts
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.