Add ability for admins to set the upload limit on projects

[theacodes]

@dstufft this is a quick implementation to make it easier for you and @ewdurbin to field upload limit size requests until #2017 is done.

I couldn't get the one test I wrote to work, I'm pretty unfamiliar with testing pyramid apps, so a little help there would be awesome. I did test this feature manually locally and it appears to work.

Stacktrace from test output:

_________________________________________________________________________________________________ TestProjectSetLimit.test_sets_integer_limit _________________________________________________________________________________________________

self = <tests.unit.admin.views.test_projects.TestProjectSetLimit object at 0x7f85b27ee828>, db_request = <pyramid.testing.DummyRequest object at 0x7f85b27f89e8>

    def test_sets_integer_limit(self, db_request):
        project = ProjectFactory.create()

        db_request.user = UserFactory.create()
        db_request.current_route_path = lambda: "/admin/projects/",
        db_request.session = pretend.stub(
            flash=pretend.call_recorder(lambda *a, **kw: None),
        )
        db_request.matchdict["project_name"] = project.normalized_name
        db_request.POST["upload_limit"] = "12345"

>       views.set_upload_limit(project, db_request)

tests/unit/admin/views/test_projects.py:360:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
warehouse/admin/views/projects.py:233: in set_upload_limit
    'admin.project.detail', project_name=project.normalized_name))
/usr/local/lib/python3.6/site-packages/pyramid/url.py:312: in route_path
    return self.route_url(route_name, *elements, **kw)
/usr/local/lib/python3.6/site-packages/pyramid/url.py:261: in route_url
    mapper = reg.getUtility(IRoutesMapper)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <Registry global>, provided = <InterfaceClass pyramid.interfaces.IRoutesMapper>, name = ''

    def getUtility(self, provided, name=u''):
        utility = self.utilities.lookup((), provided, name)
        if utility is None:
>           raise ComponentLookupError(provided, name)
E           zope.interface.interfaces.ComponentLookupError: (<InterfaceClass pyramid.interfaces.IRoutesMapper>, '')

/usr/local/lib/python3.6/site-packages/zope/interface/registry.py:286: ComponentLookupError

[dstufft]

Haven't looked at this locally, but the code looks pretty good sans a few comments.

[dstufft]

This isn't going to work if upload_limit is None. Maybe do

if not upload_limit:
    project.upload_limit = None
else:
    try:
        project.upload_limit = int(upload_limit)
    except ValueError:
        raise HTTPBadRequest(f"Invalid value for upload_limit: {upload_limit}) from None

That should cover all of the cases I think?

[theacodes]

Yep, done - without this SQLAlchemy will raise an error, but this is more explicit and raises a proper HTTP error.

[dstufft]

Yea, SQLAlchemy errors get bubbled up into Sentry and alerted whereas HTTP errors don't. So when there is an expected error we try to catch it and turn it into HTTP errors to keep sentry cleaner. Not really a super big deal here because I doubt the 2 or 3 admins are likely to start throwing a bunch of random garbage into fields, but whatever!

[theacodes]

Better safe than alerted.

[dstufft]

You'll want to add a similar thing for db_request.route_path() to fix the testing error. I normally use a pretend.call_recorder() so I can make sure that it is called with the correct arguments.

[theacodes]

Ah I get now. Thanks. Done and added two additional tests for coverage.

[theacodes]

Thanks for reviewing, @dstufft, let me know if there's anything else I need to do to get this merged.

[dstufft]

Part of me is wondering if it would make sense to deviate from the database schema here and have the input be in MB instead of bytes. It's in bytes in the DB schema because that was easier for the code, but it's kind of crummy for humans to have to manually multiple the value by 1024 * 1024 to put into this form. I think every request we've gotten has been in terms of MB so that's typically what we think of... although it might be nice to allow input like 100MB and convert that to bytes, but that's a bit more effort then just mandating it be in MB.

It also might be a good idea to flash an error and redirect back to the detail page if you try to set a limit less then the current default minimum.

[theacodes]

@dstufft all reasonable concerns, but considering this will be only used by admins and largely superseded by #2017 do you feel like it's okay as is for now?

I'm also happy to send follow-up PRs for those items to keep this one small.

[dstufft]

This would be a bit nicer as <input type="number" min={{ THE MINMUM SIZE }} ...> instead of as a text field. It might also make sense to include step="10" or something, which i think means that the arrows that browsers render this with will increment/decrement this by 10, but you can still type in whatever step you want directly into the box.

[dstufft]

I don't think it's superseded by #2017, because it's useful for Admins to be able to set this manually in either case. But I'm OK merging this as is if you can send a follow up PR with the above stuff.

[dstufft]

Oh and talked with @ewdurbin and Mark (the other two folks who do admin stuff) and they both agreed that just accepting input in MB is fine, and just hiding the fact it's being converted to bytes.

[theacodes]

Oh and talked with @ewdurbin and Mark (the other two folks who do admin stuff) and they both agreed that just accepting input in MB is fine, and just hiding the fact it's being converted to bytes.

Cool, that'll be much easier than re-writing all of the old values.

[dstufft]

Yea, basically just getting Warehouse to do the 1024 * 1024 math instead of expecting people to do it :D. We'd also convert the other way to turn bytes into MB for rendering in the form.