Arvados

review 17566-max-request-size 3cf194da907fb95f9534268af4905c6db8a71b01

In sdk/go/arvadosclient/arvadosclient_test.go

Any reason for this?

@@ -28,14 +30,12 @@ var _ = Suite(&MockArvadosServerSuite{})
 type ServerRequiredSuite struct{}

 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
-       arvadostest.StartAPI()
        arvadostest.StartKeep(2, false)
        RetryDelay = 0
 }

 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
        arvadostest.StopKeep(2)
-       arvadostest.StopAPI()
 }

why nginx.conf had to be changed in sdk/python/tests/nginx.conf to `client_max_body_size 0;`? This will affect other tests in the future I think we shall we include something in the documentation as recommended config

Since we have 128m in doc/install/install-api-server.html.textile.liquid:

  # This value effectively limits the size of API objects users can
  # create, especially collections.  If you change this, you should
  # also ensure the following settings match it:
  # * `client_max_body_size` in the previous server section
  # * `API.MaxRequestSize` in config.yml
  client_max_body_size 128m;

is unclear to me what should we recommend our users (match them both to 128m? nginx config to 0 now that we can handle it gracerfully?)

Nico César wrote:

Any reason for this?
[...]

Yes, this is: "Don't call StartAPI() from arvadosclient tests (this inadvertently caused the tests to bypass controller and communicate with RailsAPI directly)"

The test case didn't catch the bug until I did that. This probably deserves a more thorough fix though, since AFAIK bypassing controller is not desirable anywhere else, either.

why nginx.conf had to be changed in sdk/python/tests/nginx.conf to `client_max_body_size 0;`? This will affect other tests in the future I think we shall we include something in the documentation as recommended config

This lets us send a large request body through Nginx during tests. The previous config (default Nginx size limit = 1M) made it impossible to hit the default Go size limit of 10M from a test case.

Since we have 128m in doc/install/install-api-server.html.textile.liquid:
[...]

is unclear to me what should we recommend our users (match them both to 128m? nginx config to 0 now that we can handle it gracerfully?)

I think the documentation at https://doc.arvados.org/main/install/install-api-server.html is still correct:

  # This value effectively limits the size of API objects users can
  # create, especially collections.  If you change this, you should
  # also ensure the following settings match it:
  # * `client_max_body_size` in the previous server section
  # * `API.MaxRequestSize` in config.yml
  client_max_body_size 128m;

But in our test suite, we don't want to test the Nginx size limit feature, we just want it out of the way so we can exercise our own limits.

Good points.

I think is Ready to merge.