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.

Sign up for GitHub

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

Jump to bottom

Factor out common code for d2 commands register, upload and upload_docs #56378

Closed
merwok opened this issue May 24, 2011 · 12 comments
Closed

Factor out common code for d2 commands register, upload and upload_docs #56378

merwok opened this issue May 24, 2011 · 12 comments
Assignees
@merwok
Labels
easy type-feature A feature request or enhancement

Comments

@merwok
Copy link
Member

merwok commented May 24, 2011

BPO 12169
Nosy @tarekziade, @merwok
PRs
  • bpo-36188: Clean up 'unbound' method left-overs #12169
    		•
    Files
  • patch.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 = 'https://github.com/merwok'
closed_at = <Date 2011-07-08.14:34:26.059>
created_at = <Date 2011-05-24.16:48:44.541>
labels = ['easy', 'type-feature']
title = 'Factor out common code for d2 commands register, upload and upload_docs'
updated_at = <Date 2019-03-04.19:15:44.352>
user = 'https://github.com/merwok'

    bugs.python.org fields:

    activity = <Date 2019-03-04.19:15:44.352>
actor = 'mjpieters'
assignee = 'eric.araujo'
closed = True
closed_date = <Date 2011-07-08.14:34:26.059>
closer = 'eric.araujo'
components = ['Distutils2']
creation = <Date 2011-05-24.16:48:44.541>
creator = 'eric.araujo'
dependencies = []
files = ['22478']
hgrepos = []
issue_num = 12169
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['136770', '136782', '138045', '138087', '138182', '139111', '139137', '139287', '140028', '140030', '211796', '211820']
nosy_count = 6.0
nosy_names = ['tarek', 'eric.araujo', 'alexis', 'python-dev', 'John.Edmonds', 'Matthew.Iversen']
pr_nums = ['12169']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12169'
versions = ['Python 3.3']
    @merwok
    Copy link
    Member Author

    merwok commented May 24, 2011

    These three commands have different code to do POST requests, using rllib or httplib. This already made us do more work to fix bugs and to port the code. upload_docs has a top-level function for multipart encoding; this should be moved to a common module, cleaned up and used by all our code that needs a POST.

    @merwok merwok added the type-feature A feature request or enhancement label May 24, 2011
    @tarekziade
    Copy link
    Mannequin

    tarekziade mannequin commented May 24, 2011

    good idea! want to tackle this ?

    @merwok merwok added the easy label May 26, 2011
    @merwok merwok assigned merwok and unassigned tarekziade May 26, 2011
    @JohnEdmonds
    Copy link
    Mannequin

    JohnEdmonds mannequin commented Jun 10, 2011

    I'd like to try tackling this issue. I've made a patch that moves encode_multipart from upload_docs to the distutils2.command module and changed the register and upload commands to use this function.

    @merwok
    Copy link
    Member Author

    merwok commented Jun 10, 2011

    Thanks for your contribution. Unfortunately, I used “distutils2” as a familiar name for what is now known as the packaging module, in the 3.3 standard library. This document should help you find the right codebase to work from: http://wiki.python.org/moin/Distutils/Contributing

    Regarding the new location of the moved code, I think the util module would be a better choice than command.

    @JohnEdmonds
    Copy link
    Mannequin

    JohnEdmonds mannequin commented Jun 11, 2011

    Here is the patch, re-written for the packaging module.

    @merwok
    Copy link
    Member Author

    merwok commented Jun 25, 2011

    I made some comments on the code review site; maybe you didn’t get the email, there is a known bug about that. You can follow the “review” link on the right of the patch file to see it.

    I have closed bpo-10510, which asked that HTTP request use CRLF for maximum compatibility. The current patch does it.

    @JohnEdmonds
    Copy link
    Mannequin

    JohnEdmonds mannequin commented Jun 26, 2011

    Thanks for reviewing the patch. I don't believe I received an email for the review. I think I have addressed your comment about the usage of str(body) by removing the call to str() and changing the tests to use byte literals.

    As for the content-length changing in the tests, that is because encode_multipart uses '\r\n' to end lines while the previous code only ended lines in '\n'.

    @merwok
    Copy link
    Member Author

    merwok commented Jun 27, 2011

    Great patch, thanks! It’s on the top of my commit list.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 8, 2011

    New changeset c2785ed52ed4 by Éric Araujo in branch 'default':
    Factor out code used by packaging commands for HTTP requests (bpo-12169).
    http://hg.python.org/cpython/rev/c2785ed52ed4

    @merwok
    Copy link
    Member Author

    merwok commented Jul 8, 2011

    I added a few blank lines, changed the error message when boundary is not bytes, moved test_encode_multipart to test_util.py and committed.

    Thanks again!

    @merwok merwok closed this as completed Jul 8, 2011
    @MatthewIversen
    Copy link
    Mannequin

    MatthewIversen mannequin commented Feb 21, 2014

    Hi, I'm wondering why this branch was never merged in?

    AFIAK, it's roundabout here - http://hg.python.org/cpython/log/28e4cd8fd864/Lib/packaging/command/upload.py

    It'd be great to have distutils submit forms that are compliant with the MIME spec (in my use case, cherrypy completely borks on it).

    @merwok
    Copy link
    Member Author

    merwok commented Feb 21, 2014

    Hi, I'm wondering why this branch was never merged in?

    It was, see the comment before with the changeset link.

    AFIAK, it's roundabout here - http://hg.python.org/cpython/log/28e4cd8fd864/Lib/packaging/command/upload.py

    I don’t understand what you mean.

    It'd be great to have distutils submit forms that are compliant with the MIME spec (in my use case,
    cherrypy completely borks on it).

    This issue was about distutils2/packaging, a project which is now abandoned. If a distutils command has a bug with HTTP POSTs, please file another ticket and I’ll have a look. Thanks in advance!

    @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
    Assignees

    @merwok merwok

    Labels
    easy type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @merwok