twisted.web.server.Request.view_setResponseCode doesn't accept message parameter

[tom.prince]

Add missing message parameter

[Fahrenheit]

Thanks for the patch! Comments below:

1. twisted/web/server.py
   1. Not a huge deal, but the function signature for view_setResponseCode should match that of setResponseCode, so maybe remove the whitespace around the '='.
2. twisted/web/test/test_distrib.py
   1. Again, not a huge deal, but the second underscore in test_setResponseCode_message doesn't match the twisted guidelines; everything after "test_" should be camelcase, rather that separated by any underscores.
   2. Last issue, and this one matters, is that your test doesn't actually make sure that view_setResponseCode does the correct thing with the message argument; I was able to remove it from your implementation and the test still passed, so make sure that your test checks that the message gets used properly.

You should be good once you fix up the test and the style stuff would be nice too, while you're at it. Thanks again!

[tom.prince]

patch v2

[tom.prince]

I think this fixes all issues. The old _requestTest used getPage, which didn't have a way of looking at the response. This changes the tests to use Agent.

[jesstess]

Thanks for working on this, tom.prince. A few comments:

• Since view_setResponseCode is in twisted.web.server, it seems like tests for this change would go in twisted.web.test.test_web with the other server tests. Can you explain why they should be in test_distrib, or move them to test_web? There's a test_setResponseCode in test_http you could crib from. It would also allow you to construct a more succinct unit test than one that is forcing you to make changes in test_largeWrite and test_largeReturn.
• If your changes stay in test_distrib, there are a couple of typos:
  • The test no longer uses getPage:

            @param **kwargs: Extra keyword arguments to pass to L{getPage} when
    

  • consiting ==> consisting in:

            @return: A L{Deferred} which fires with a tuple consiting of a                                                                                                                                           
    

  • IResposne ==> IResponse in:

                body of the response and an L{IResposne} with the response itself.
    

Sorry for the back and forth, and thanks again for working on this!

[exarkun]

Since view_setResponseCode is in twisted.web.server, it seems like tests for this change would go in twisted.web.test.test_web

The strange factoring of the implementation of distrib means that the test organization differs a bit from the usual approach. I think test_distrib is the right place for these tests, because that's where there's testing infrastructure for actually exercising the distrib system. That involves setting up a server and connecting a distrib client to it (which would be nice to avoid, but that's how all the existing distrib tests are written...).

[tom.prince]

Patch v3.

[tom.prince]

The typos have been fixed. I also split _requestTest, so that creating the servers is separate from making the request, allowing the existing tests to remain unchanged.

[exarkun]

(In [33876]) Branching to 'setresponsecode-distrib-5525'

[exarkun]

(In [33877]) Apply distrib_server_response_code_message_3.patch

refs #5525

[exarkun]

Thanks. This is looking pretty great now. Just a few minor points:

1. Methods in a class should be separated from each other by two blank lines
2. There are some tabs in the patch; there should never be tabs in a .py file.
3. The change needs a news fragment. This is a nice bug fix for distrib servers, so it's worth mentioning in a .bugfix fragment.

These are pretty trivial points. If I get a clean build on buildbot, I'll deal with these changes and merge. Thanks again!

[exarkun]

 Build results

[exarkun]

(In [33885]) Mege setresponsecode-distrib-5525

Author: tom.prince Reviewer: Fahrenheit, jesstess, exarkun Fixes: #5525

Add the message parameter to the PB view of twisted.web.server.Request. This allows distributed servers (twisted.web.distrib) to set the response message, just as any normal HTTP code in a Twisted Web server may do.