Page MenuHomePhabricator

Site.redirect deprecated argument usage can cause exception
Closed, ResolvedPublic

Description

redirect.py fails with -moves option:

C:\pwb\core>pwb.py redirect.py do -always -moves
Retrieving all moved pages via API...
............................

Rosetta-Lander <<<

Links to: [[Philae (Sonde)]].
Links to: [[Philae]].

Traceback (most recent call last):

File "C:\pwb\core\pwb.py", line 181, in <module>
  run_python_file(fn, argv, argvu)
File "C:\pwb\core\pwb.py", line 75, in run_python_file
  exec(compile(source, filename, "exec"), main_mod.__dict__)
File "C:\pwb\core\scripts\redirect.py", line 821, in <module>
  main()
File "C:\pwb\core\scripts\redirect.py", line 818, in main
  bot.run()
File "C:\pwb\core\scripts\redirect.py", line 721, in run
  self.fix_double_redirects()
File "C:\pwb\core\scripts\redirect.py", line 528, in fix_double_redirects
  self.fix_1_double_redirect(redir_name)
File "C:\pwb\core\scripts\redirect.py", line 665, in fix_1_double_redirect
  '#%s %s' % (self.site.redirect(True),
File "C:\pwb\core\pywikibot\tools.py", line 647, in wrapper
  return obj(*__args, **__kw)

TypeError: redirect() takes exactly 1 argument (2 given)
<type 'exceptions.TypeError'>
CRITICAL: Waiting for 1 network thread(s) to finish. Press ctrl-c to abort

C:\pwb\core>

corresponding version is:

C:\pwb\core>pwb.py version
Pywikibot: pywikibot-core (f12709d, s5607, 2014/11/16, 00:47:43, ok)
Release version: 2.0b2
httplib2 version: 0.9

cacerts: C:\pwb\core\externals\httplib2\python2\httplib2\cacerts.txt
  certificate test: ok

Python: 2.7.3 (default, Apr 10 2012, 23:24:47) [MSC v.1500 64 bit (AMD64)]

unicode test: ok

Older code does still the job well:

c:\Pywikipedia\ssh\pywikibot\core>pwb.py version.py
Pywikibot: [ssh] pywikibot-core (dfdc0c9, g4462, 2014/11/03, 07:43:21, OUTDATED)

Release version: 2.0b2
httplib2 version: 0.9

cacerts: C:\Pywikipedia\ssh\pywikibot\core\externals\httplib2\python2\httplib2

\cacerts.txt

certificate test: ok

Python: 2.7.3 (default, Apr 10 2012, 23:24:47) [MSC v.1500 64 bit (AMD64)]

unicode test: ok

c:\Pywikipedia\ssh\pywikibot\core>


Version: core-(2.0)
Severity: major

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:47 AM
bzimport set Reference to bz73489.
bzimport added a subscriber: Unknown Object (????).

Seems the decorator does not handle arguments for redirect as expected:

import pwb
import pywikibot as py
s = py.Site()
x = s.redirect()
x

u'WEITERLEITUNG'

x = s.redirect(True)

Traceback (most recent call last):

File "<pyshell#5>", line 1, in <module>
  x = s.redirect(True)
File "pywikibot\tools.py", line 647, in wrapper
  return obj(*__args, **__kw)

TypeError: redirect() takes exactly 1 argument (2 given)

The decorator only handles keyword arguments. I had a patch for positional arguments although that made it considerably more complex: https://gerrit.wikimedia.org/r/155020/

So the script could be fixed, but unfortunately other scripts won't be compatible if they use the argument positionally.

Change 173659 had a related patch set uploaded by XZise:
[FIX] Don't use deprecated parameter of redirect()

https://gerrit.wikimedia.org/r/173659

Change 173659 merged by jenkins-bot:
[FIX] Don't use deprecated parameter of redirect()

https://gerrit.wikimedia.org/r/173659

This patch should fix your problem but still: Do we want to ignore that compat scripts might break because of this?

@Fabian, IMO this provides a good use case to make use of your code for deprecating positional arguments. As positional arguments are a lot more error prone to play with, we may want to build a few specific tools (that cant-be/shouldnt-be used with other deprecators) rather than one magical tool which is very complex and error prone if mixed with other deprecators/decorators.

I guess we should not ignore it without any hint. What about the other methods where the arguments are obsolete with the previous patch?

What about them? In our code base are no callers of those methods or they don't use the deprecated parameter (at least git grep "\.…(" told me). But obviously for a later patch which avoids breakage those should be included as well.

Change 174107 had a related patch set uploaded by XZise:
[FIX] Allow deprecation of all parameters

https://gerrit.wikimedia.org/r/174107

Change 174107 had a related patch set uploaded (by XZise):
[FIX] Allow deprecation of trailing parameters

https://gerrit.wikimedia.org/r/174107

Patch-For-Review

Change 174107 merged by jenkins-bot:
[FIX] Allow deprecation of trailing parameters

https://gerrit.wikimedia.org/r/174107

jayvdb claimed this task.
jayvdb set Security to None.
jayvdb removed a subscriber: Unknown Object (????).