Page MenuHomeSoftware Heritage

Web API: /known/ input size limit
ClosedPublic

Authored by DanSeraf on Feb 12 2020, 3:17 PM.

Details

Summary

The number of PID's the /known/ endpoint can receive is specified in swh.web.config.
If the limit is exceeded, an InputSizeExc is raised with the error code 413.

Related (T2276)

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Don't worry about the test failures, they are not related to your changes. I just restarted them.

vlorentz added inline comments.
swh/web/config.py
50 ↗(On Diff #9513)

You don't need to make this configurable; most API endpoints aren't. (eg. look for limit in swh/web/api/views/origin.py)

This revision now requires changes to proceed.Feb 12 2020, 3:33 PM
swh/web/config.py
50 ↗(On Diff #9513)

So, should i limit the request directly inside the endpoint?

DanSeraf marked an inline comment as not done.Feb 12 2020, 3:41 PM
swh/web/config.py
50 ↗(On Diff #9513)

Yes, like you already do, it just doesn't need a config value

anlambert requested changes to this revision.EditedFeb 12 2020, 3:43 PM
anlambert added a subscriber: anlambert.

A new test should be added to check the 413 code is returned when the input pid list is too large..

swh/web/common/exc.py
46

LargePayloadExc seems a more generic name to me.

swh/web/config.py
50 ↗(On Diff #9513)

Adding this in configuration is not a shocker to me.

pids_list_max_size is also more explicit.

swh/web/config.py
50 ↗(On Diff #9513)

It would be hard to test without a config value, i can't get the limit from the endpoint

swh/web/config.py
50 ↗(On Diff #9513)

Alright. But use a reasonable default value then (eg. 1000 or 10000)

zack added inline comments.
swh/web/config.py
50 ↗(On Diff #9513)

You don't need to make this configurable; most API endpoints aren't.

well, this isn't great, they should be configurable :-)

(but ack, I agree it is pointless to diverge and make only one of them configurable at this point)

making all endpoints configurable might be a good easy hack task to file

limit is set inside the endpoint

This revision is now accepted and ready to land.Feb 14 2020, 4:10 PM
This revision was landed with ongoing or failed builds.Feb 17 2020, 11:54 AM
This revision was automatically updated to reflect the committed changes.