api.server: Fix client_max_size parameter transmission to aiohttp
ClosedPublic
Actions

Authored by anlambert on Apr 26 2019, 4:15 PM.

Details

Reviewers

douardda

Group Reviewers

Reviewers

Commits

rDVAU7f05bda76a4a: api.server: Fix client_max_size parameter transmission to aiohttp
rDVAU9d146a5c0180: api.server: Move config parsing in make_app_from_configfile

Summary

Since a recent refactoring, the client_max_size config value was no more
transmitted to the aiohttp.web.Application instance.

This was resulting in errors when a cooked archive had a size greater than 1048576 bytes,
see stack trace observed when playing with the vault in docker-compose environment below:

swh-vault-worker_1            | [2019-04-26 13:05:13,855: INFO/MainProcess] Connected to amqp://guest:**@amqp:5672//
swh-vault-worker_1            | [2019-04-26 13:05:13,988: INFO/MainProcess] vault@2713bd8ecd80 ready.
swh-vault-worker_1            | [2019-04-26 13:11:42,416: INFO/MainProcess] Received task: swh.vault.cooking_tasks.SWHCookingTask[2cf11e3e-1005-428e-8c1d-0d181c3f2623]  
swh-vault-worker_1            | [2019-04-26 13:11:42,455: INFO/ForkPoolWorker-1] Loading config file /cooker.yml
swh-vault-worker_1            | [2019-04-26 13:19:21,483: ERROR/ForkPoolWorker-1] Bundle cooking failed.
swh-vault-worker_1            | Traceback (most recent call last):
swh-vault-worker_1            |   File "/srv/softwareheritage/venv/lib/python3.6/site-packages/swh/vault/cookers/base.py", line 122, in cook
swh-vault-worker_1            |     self.backend.put_bundle(self.CACHE_TYPE_KEY, self.obj_id, bundle)
swh-vault-worker_1            |   File "/srv/softwareheritage/venv/lib/python3.6/site-packages/swh/vault/api/client.py", line 45, in put_bundle
swh-vault-worker_1            |     data=bundle)
swh-vault-worker_1            |   File "/srv/softwareheritage/venv/lib/python3.6/site-packages/swh/core/api/__init__.py", line 198, in post
swh-vault-worker_1            |     return self._decode_response(response)
swh-vault-worker_1            |   File "/srv/softwareheritage/venv/lib/python3.6/site-packages/swh/core/api/__init__.py", line 235, in _decode_response
swh-vault-worker_1            |     response.content,
swh-vault-worker_1            | swh.core.api.RemoteException: Unexpected status code for API request: 413 (b'Maximum request body size 1048576 exceeded, actual body size 1272960')

Diff Detail

Repository

rDVAU Software Heritage Vault

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

anlambert created this revision.Apr 26 2019, 4:15 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 26 2019, 4:15 PM

Build is green
See https://jenkins.softwareheritage.org/job/DVAU/job/tox/63/ for more details.

Harbormaster completed remote builds in B5545: Diff 4664.Apr 26 2019, 4:18 PM

anlambert mentioned this in D1436: tests: Add integration tests for the Software Heritage stack.May 6 2019, 11:21 AM

Shouldn't the client_max_size be added to DEFAULT_CONFIG instead?

Hmm, it is already. Shouldn't cfg['client_max_size'] always work, then?

I know it is already set in default config but I preferred to explicitly set a good default value in case where a config file is used and is missing the client_max_size entry (as Idid in swh-objstorage here : rDOBJSc334336b3ea7a068efa1dfe9c667349eef02f57c).
We met so many errors by the past when this important parameter is not provided to aiohttp, so we should have a fallback value to avoid them.

Forget my last comment, reading the value using cfg['client_max_size'] is indeed enough here. I will update accordingly.

Update: client_max_size is already set with a good default value in default configuration, no need to duplicate it

Build is green
See https://jenkins.softwareheritage.org/job/DVAU/job/tox/65/ for more details.

Harbormaster completed remote builds in B5580: Diff 4696.May 6 2019, 2:46 PM

Not a big stuff, but I would have preferred to see the refactoring (of the get_local_backend's code) and the actual fix (add client_max_size arg when calling make_app) in 2 revisions.

This revision is now accepted and ready to land.May 7 2019, 2:32 PM

Update: separate introduced changes into two different commits

Build is green
See https://jenkins.softwareheritage.org/job/DVAU/job/tox/66/ for more details.

Harbormaster completed remote builds in B5618: Diff 4725.May 7 2019, 2:45 PM

Closed by commit rDVAU9d146a5c0180: api.server: Move config parsing in make_app_from_configfile (authored by anlambert). · Explain WhyMay 7 2019, 2:45 PM

This revision was automatically updated to reflect the committed changes.