Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow urllib.request.Request subclasses to override method #63178

Closed
jaraco opened this issue Sep 8, 2013 · 8 comments
Closed

Allow urllib.request.Request subclasses to override method #63178

jaraco opened this issue Sep 8, 2013 · 8 comments
Labels
stdlib Python modules in the Lib dir

Comments

@jaraco
Copy link
Member

jaraco commented Sep 8, 2013

BPO 18978
Nosy @jaraco, @orsenthil
Files
  • 6d6d68c068ad.diff
  • 2b2744cfb08f.diff
  • 061eb75339e2.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2013-09-22.14:11:15.600>
    created_at = <Date 2013-09-08.16:41:28.700>
    labels = ['library']
    title = 'Allow urllib.request.Request subclasses to override method'
    updated_at = <Date 2014-02-25.15:23:30.204>
    user = 'https://github.com/jaraco'

    bugs.python.org fields:

    activity = <Date 2014-02-25.15:23:30.204>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2013-09-22.14:11:15.600>
    closer = 'python-dev'
    components = ['Library (Lib)']
    creation = <Date 2013-09-08.16:41:28.700>
    creator = 'jaraco'
    dependencies = []
    files = ['31676', '31677', '31678']
    hgrepos = ['208']
    issue_num = 18978
    keywords = ['patch']
    message_count = 8.0
    messages = ['197281', '197283', '197284', '197288', '198278', '198280', '198551', '212184']
    nosy_count = 3.0
    nosy_names = ['jaraco', 'orsenthil', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue18978'
    versions = ['Python 3.4']

    @jaraco
    Copy link
    Member Author

    jaraco commented Sep 8, 2013

    In Python 2.x and 3.2, I used to use a Request subclass I created for overriding the method used:

    class MethodRequest(request.Request):
    	def __init__(self, *args, **kwargs):
    		"""
    		Construct a MethodRequest. Usage is the same as for
    		`urllib.request.Request` except it also takes an optional `method`
    		keyword argument. If supplied, `method` will be used instead of
    		the default.
    		"""
    		if 'method' in kwargs:
    			self.method = kwargs.pop('method')
    		return request.Request.__init__(self, *args, **kwargs)
    
    	def get_method(self):
    		return getattr(self, 'method', request.Request.get_method(self))

    In Python 3.3, which now supports a method parameter, it broke this paradigm (because the method is stored in the instance and is always set to None in __init__ if not specified).

    I believe a paradigm where the method is stored as a class attribute and possibly overridden in an instance would be much better, allowing for subclasses to simply and directly override the method. For example:

    class HeadRequest(MethodRequest):
    	method = 'HEAD'

    That straightforward example works very well if method is allowed to be a class attribute, but won't work at all if 'method' is always set as an instance attribute in __init__.

    And while it's possible for HeadRequest to override __init__, that requires HeadRequest to override that entire signature, which is less elegant than simply setting a class attribute.

    For Python 3.4, I'd like to adapt the Request class to allow the Method to be defined at the class level (while still honoring customization at the instance level).

    @jaraco jaraco added the stdlib Python modules in the Lib dir label Sep 8, 2013
    @orsenthil
    Copy link
    Member

    Hi Jason,

    Agree with you. This design change could be valuable in extending urllib.request.Request

    Thanks!

    @jaraco
    Copy link
    Member Author

    jaraco commented Sep 8, 2013

    I've created a clone in which to draft this work.

    @jaraco
    Copy link
    Member Author

    jaraco commented Sep 8, 2013

    I've added tests to capture the new behavior.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 22, 2013

    New changeset 6d6d68c068ad by Jason R. Coombs in branch 'default':
    Issue bpo-18978: Allow Request.method to be defined at the class level.
    http://hg.python.org/cpython/rev/6d6d68c068ad

    New changeset 2b2744cfb08f by Jason R. Coombs in branch 'default':
    Issue bpo-18978: A more elegant technique for resolving the method
    http://hg.python.org/cpython/rev/2b2744cfb08f

    New changeset 061eb75339e2 by Jason R. Coombs in branch 'default':
    Issue bpo-18978: Add tests to capture expected behavior for class-level method overrides.
    http://hg.python.org/cpython/rev/061eb75339e2

    New changeset 8620aea9bbca by Jason R. Coombs in branch 'default':
    Close bpo-18978: Merge changes.
    http://hg.python.org/cpython/rev/8620aea9bbca

    @python-dev python-dev mannequin closed this as completed Sep 22, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 22, 2013

    New changeset 7f13d5ecf71f by Jason R. Coombs in branch 'default':
    Issue bpo-18978: Update docs to reflect explicitly the ability to set the attribute at the class level.
    http://hg.python.org/cpython/rev/7f13d5ecf71f

    @orsenthil
    Copy link
    Member

    Thanks for the this change, Jason. Docs could be updated to reflect this change (using ..versionchanged: directive). Thank you!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 25, 2014

    New changeset 1afbd851d1c1 by R David Murray in branch 'default':
    whatsnew: Request.method can be overridden in subclasses (bpo-18978).
    http://hg.python.org/cpython/rev/1afbd851d1c1

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants