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

Details

Reference
bz73489
Related Gerrit Patches:

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 (????).
Xqt created this task.Nov 16 2014, 3:21 PM
Xqt added a comment.Nov 16 2014, 3:38 PM

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)

XZise added a comment.Nov 16 2014, 3:47 PM

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

XZise added a comment.Nov 16 2014, 4:32 PM

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.

Xqt added a comment.Nov 16 2014, 4:51 PM

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

XZise added a comment.Nov 16 2014, 4:54 PM

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 closed this task as Resolved.Nov 25 2014, 1:14 AM
jayvdb claimed this task.
jayvdb reassigned this task from jayvdb to XZise.Nov 25 2014, 12:15 PM
jayvdb set Security to None.
jayvdb removed a subscriber: Unknown Object (????).