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

Deprecate addinfourl getters #56916

Closed
ezio-melotti opened this issue Aug 7, 2011 · 14 comments
Closed

Deprecate addinfourl getters #56916

ezio-melotti opened this issue Aug 7, 2011 · 14 comments
Assignees
Labels
3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ezio-melotti
Copy link
Member

BPO 12707
Nosy @orsenthil, @ezio-melotti, @merwok, @sandrotosi, @akheron, @berkerpeksag, @vadmium, @demianbrecht, @matrixise
PRs
  • bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447
  • bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447
  • bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447
  • bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl #11447
  • 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 = 'https://github.com/orsenthil'
    closed_at = <Date 2019-09-13.11:40:45.217>
    created_at = <Date 2011-08-07.23:46:10.019>
    labels = ['easy', 'type-feature', 'library', '3.9']
    title = 'Deprecate addinfourl getters'
    updated_at = <Date 2019-09-13.11:40:45.217>
    user = 'https://github.com/ezio-melotti'

    bugs.python.org fields:

    activity = <Date 2019-09-13.11:40:45.217>
    actor = 'matrixise'
    assignee = 'orsenthil'
    closed = True
    closed_date = <Date 2019-09-13.11:40:45.217>
    closer = 'matrixise'
    components = ['Library (Lib)']
    creation = <Date 2011-08-07.23:46:10.019>
    creator = 'ezio.melotti'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 12707
    keywords = ['patch', 'patch', 'patch', 'patch', 'easy']
    message_count = 14.0
    messages = ['141756', '141757', '142424', '142438', '147983', '180901', '180902', '184552', '184553', '184567', '234945', '234947', '352293', '352294']
    nosy_count = 10.0
    nosy_names = ['orsenthil', 'nadeem.vawda', 'ezio.melotti', 'eric.araujo', 'sandro.tosi', 'petri.lehtinen', 'berker.peksag', 'martin.panter', 'demian.brecht', 'matrixise']
    pr_nums = ['11447', '11447', '11447', '11447']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue12707'
    versions = ['Python 3.9']

    @ezio-melotti
    Copy link
    Member Author

    The documentation for urllib.request.urlopen 0 says that:
    """
    This function returns a file-like object [addinfourl] with two additional methods from the urllib.response module
    geturl() — return the URL of the resource retrieved, [...]
    info() — return the meta-information of the page, [...]
    """

    There's also a third undocumented method: getcode(). Looking at the code1 ISTM that the 3 getters (geturl(), info(), and getcode()):

    1. have an inconsistent interface (why not getinfo() or getheaders()?);
    2. they just return the url, headers, and code attributes;

    For these two reasons I propose to:

    • document the 3 attributes as the suggested way to access this information;
    • deprecate the 3 getters;
    • avoid to document the now undocumented getcode();

    @ezio-melotti ezio-melotti added stdlib Python modules in the Lib dir easy labels Aug 7, 2011
    @ezio-melotti
    Copy link
    Member Author

    Another possible improvement that can be done is providing a proper Response object instead of addbase, addinfo, addinfourl (yes, those are oddly-named classes, not functions 0).
    The Response object could be like 'addbase', but with this signature:

    class Response:
        def __init__(self, fp, headers=None, url=None, code=None):
            ...
    and without additional getters.

    The addclosehook class could be provided via an alternative constructor:

        @classmethod
        def with_close_hook(cls, fp, hook, *hookargs):
            ...

    @merwok
    Copy link
    Member

    merwok commented Aug 19, 2011

    For these two reasons I propose to:

    • document the 3 attributes as the suggested way to access this
      information;
    • deprecate the 3 getters;
    • avoid to document the now undocumented getcode();
      +1

    The addclosehook class could be provided via an alternative constructor
    I can’t say why, but I don’t like that.

    @ezio-melotti
    Copy link
    Member Author

    I thought about having another class, but I couldn't come up with a decent name for it (ResponseWithCloseHook?). After all it's still a Response and unless you need a way to distinguish responses with and without close hooks, I think it might be better to have a single class for both.

    @merwok
    Copy link
    Member

    merwok commented Nov 20, 2011

    I thought about having another class, but I couldn't come up with a
    decent name for it (ResponseWithCloseHook?).

    If it’s used together with the base Response class (I don’t have the details in memory anymore), you could try ClosingMixin or FileLikeMixin.

    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Sep 26, 2012
    @akheron
    Copy link
    Member

    akheron commented Jan 29, 2013

    +1 for the documentation changes, which should be applied to 2.7 as well. The deprecation is the only thing to go to 3.4 only, if it's done at all.

    @akheron
    Copy link
    Member

    akheron commented Jan 29, 2013

    Also note that getcode() is already documented in urllib (not urllib2) documentation:

    http://docs.python.org/2/library/urllib.html#urllib.urlopen
    

    @orsenthil
    Copy link
    Member

    And getcode is documented all 3.x documentation now.

    URLOpener (and by inheritance) raise DeprecationWarning since 3.3. I believe, when class becomes deprecated and removed, their methods go away too? For the URLOpener, I would like that to happen as it will present a much cleaner newer way for further enhancement.

    @orsenthil
    Copy link
    Member

    URLOpener (and by inheritance *FancyURLOpener*)

    @orsenthil orsenthil self-assigned this Mar 18, 2013
    @orsenthil
    Copy link
    Member

    This commit by Ezio in the tests is related (1bcddc0a3765)

    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2015

    I think it would be okay to deprecate the methods in the documentation, but they should not be removed nor trigger warnings any time soon.

    Currently the following related methods and attributes are documented:

    • addinfourl.getcode() == HTTPResponse.status == HTTPError.code
    • HTTPResponse.reason == HTTPError.reason (HTTPError documentation is vague on this.)
    • addinfourl.geturl()
    • addinfourl.info() == HTTPResponse.msg == HTTPError.headers (But see bpo-22989 and patch in bpo-21228 for a quirk with HTTPResponse.)

    It would be nice to ensure these three classes all implement the same “Response” interface, except that geturl() and “url” may not be appropriate for HTTPResponse. The “code” and “headers” attributes should definitely be documented for consistency. I propose also adding and documenting “reason”.

    @vadmium
    Copy link
    Member

    vadmium commented Jan 29, 2015

    Blessing a geturl() method or “url” attribute on HTTPError might require bpo-13567 to be fixed

    @matrixise matrixise added the 3.9 only security fixes label Sep 11, 2019
    @matrixise
    Copy link
    Member

    New changeset ff2e182 by Stéphane Wirtel (Ashwin Ramaswami) in branch 'master':
    bpo-12707: deprecate info(), geturl(), getcode() methods in favor of headers, url, and status properties for HTTPResponse and addinfourl (GH-11447)
    ff2e182

    @matrixise
    Copy link
    Member

    Thank you so much for your contribution, I close this issue and have merged the PR.

    @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
    3.9 only security fixes easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants