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

Added help entry for File already exists error #2997

Merged
merged 5 commits into from Feb 26, 2018

Conversation

waseem18
Copy link
Contributor

@waseem18 waseem18 commented Feb 20, 2018

Fixes #2701.

Need to add link to help entry in the the error message. I guess this change goes into twine.

@di
Copy link
Member

di commented Feb 20, 2018

The error message is here, we can update it to include a link to this help section.

@waseem18
Copy link
Contributor Author

Sure @di Will do

<section id="file-name-reuse" class="common-question">
<h2>{{ file_name_reuse() }}</h2>
<p>
The error <i>HTTPError: 400 Client Error: File already exists</i> is caused because of one of the below two reasons:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you rephrase as:

The error HTTPError: 400 Client Error: File already exists happens for one of two reasons:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - Will rephrase

di
di previously requested changes Feb 20, 2018
@@ -1033,7 +1036,8 @@ def file_upload(request):
raise _exc_with_message(
HTTPBadRequest,
"This filename has previously been used, you should use a "
"different version.",
"different version. "
"See https://pypi.org/help/#file-name-reuse",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you generate these URLs with request.route_url('help', _anchor='file-name-reuse') instead? Otherwise these will be wrong for https://test.pypi.org/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, Will address.

Copy link
Contributor Author

@waseem18 waseem18 Feb 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@di What about _scheme and _host ?

request.route_url('help', _anchor='file-name-reuse') generates only path right. What about host?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

route_path would generate a path, like /help/#file-name-reuse, route_url generates a fully qualified URL, like https://pypi.org/help/#file-name-reuse, so you don't need to specify a host or scheme.

@waseem18
Copy link
Contributor Author

@di After I put route_url I'm getting below error when I run make tests. I tried a lot to know the cause of it and resolve but in vain. Could you please help me with this?

error

@di
Copy link
Member

di commented Feb 23, 2018

@waseem18 The issue is that your tests don't mock out db_request.route_url, you'll need to add something like:

db_request.route_url = pretend.call_recorder(lambda route, **kw: "/the/help/url/")

And then your assertion will become:

assert resp.status == (
    "400 This filename has previously been used, you should use a "
    "different version. See /the/help/url/"
) 

since we don't actually need to test if route_url works as it should.

@di
Copy link
Member

di commented Feb 23, 2018

And, additionally, you can test that route_url was called as expected:

assert db_request.route_url.calls == [pretend.call('help', _anchor='file-name-reuse')]

@brainwane
Copy link
Contributor

@waseem18 I thought you might want some tips on tidying your commit history, including updating your commit messages. And when you update your code or your commit message, you can update your pull request and the changes will appear here. :) (I love the Zulip docs for these tips!)

@waseem18
Copy link
Contributor Author

Thanks for the links @brainwane
I'll surely follow 👍

@di di dismissed their stale review February 26, 2018 14:50

Addressed.

@di di merged commit 8a82f22 into pypi:master Feb 26, 2018
@di
Copy link
Member

di commented Feb 26, 2018

Thanks @waseem18!

@waseem18 waseem18 deleted the file-reuse-documentation branch February 26, 2018 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants