Add One-Shot Upload #246
Add One-Shot Upload #246
Conversation
|
Hello @CodeHeeler! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-07-31 13:50:30 UTC |
CodeHeeler
force-pushed
the
issue-4396
branch
2 times, most recently
from
a2bdf0b
to
e384f06
Compare
CodeHeeler
force-pushed
the
issue-4396
branch
3 times, most recently
from
2f74cdb
to
40a98d5
Compare
| 'repository': repo['_href']}) | ||
| task = self.client.get(task_url['task']) | ||
| new_repo_version = task['created_resources'][0] | ||
| version_content_query = self.client.get(new_repo_version)[ |
There was a problem hiding this comment.
If this can be split up differently somehow, that would be awesome. I know it's a nitpick but this is hard to read to me.
There was a problem hiding this comment.
Would it help to add a comment saying what we're trying to access and that it's deeply nested?
There was a problem hiding this comment.
How about
version_content_query = self.client.get(
new_repo_version
)['content_summary']['added']['python.python']['href']
There was a problem hiding this comment.
Done. I like that much better. Honestly, with as large as screens are now, I wish PEP8 would ease the character limit per line a bit more because I spend so much time making things harder to read to get around it.
There was a problem hiding this comment.
Yup, we've historically used 100 as our limit instead of 80
CHANGES/4396.feature
Outdated
| @@ -0,0 +1 @@ | |||
| Add one-shot upload and accompanying tests and docs | |||
There was a problem hiding this comment.
No need to mention tests in the changelog. IMO this should just be stuff we want users to know.
docs/workflows/upload.rst
Outdated
|
|
||
| Create content from an artifact | ||
| ------------------------------- | ||
| Add content to a repository |
There was a problem hiding this comment.
s/$/during one-shot upload/
pulp_python/app/viewsets.py
Outdated
| break | ||
| if python_models.PythonPackageContent.objects.filter(filename=filename): | ||
| raise serializers.ValidationError(detail={'filename': _('This field must be unique')}) | ||
| # TODO: change from 400 to 409 |
There was a problem hiding this comment.
File an issue for this, remove the TODO.
pulp_python/app/serializers.py
Outdated
| view_name='repositories-detail', | ||
| ) | ||
| file = serializers.FileField( | ||
| help_text=_("The python file."), |
There was a problem hiding this comment.
Might be helpful to give a bit more detail here. ie, a whl or whatever-- this sounds like mything.py could be valid.
| if filename.endswith(ext): | ||
| # Copy file to a temp directory under the user provided filename, we do this | ||
| # because pkginfo validates that the filename has a valid extension before | ||
| # reading it |
There was a problem hiding this comment.
Does it? I know this came from the RPM plugin which did, I know nothing about pkginfo.
There was a problem hiding this comment.
Yes, pkginfo does, I verified it in the python shell.
CodeHeeler
force-pushed
the
issue-4396
branch
2 times, most recently
from
f03ff82
to
48e88db
Compare
Adds one-shot upload feature, optionally specifying a repo Updates tests and docs accordingly fixes #4396 https://pulp.plan.io/issues/4396
Add one-shot upload with associated docs and tests
One-shot upload with optionally specified repository
fixes #4396
https://pulp.plan.io/issues/4396