Change GET request to POST requests #2732
Conversation
Get request should not have http body according to RFC.
Works well with POST here, and fails with GET for some cases:
2019-03-22 20:51:24,589 tornado.application[19] ERROR: Uncaught exception GET /api/add_task ...
Traceback (most recent call last):
File "contrib/python/tornado/tornado/web.py", line 1510, in _execute
result = method(*self.path_args, **self.path_kwargs)
File "contrib/python/luigi/luigi/server.py", line 120, in get
result = getattr(self._scheduler, method)(**arguments)
File "contrib/python/luigi/luigi/scheduler.py", line 819, in add_task
assert worker is not None
AssertionError
By the way: urllib fetcher uses POST here (urlopen(full_url, body, timeout).)
|
Please disregard my comment, we already do this: https://github.com/spotify/luigi/blob/master/luigi/server.py#L129 |
|
It is not a breaking change as I can see. |
|
Makes sense I suppose - honestly I don't know. Can you just clarify when one gets the stack trace? For particular version of libraries or is it your task that is special? |
|
I guess the thing is in third-party http-balancer that stands between luigi client and scheduler in my infrastructure. Maybe the balancer drops the http body. As far as I know it does not reproduce with http connection. |
|
Thanks for this patch @peter-volkov ! |
GET requests should not have http body according to RFC.
Works well with POST here, and fails with GET for some cases:
2019-03-22 20:51:24,589 tornado.application[19] ERROR: Uncaught exception GET /api/add_task ...
Traceback (most recent call last):
File "contrib/python/tornado/tornado/web.py", line 1510, in _execute
result = method(*self.path_args, **self.path_kwargs)
File "contrib/python/luigi/luigi/server.py", line 120, in get
result = getattr(self._scheduler, method)(**arguments)
File "contrib/python/luigi/luigi/scheduler.py", line 819, in add_task
assert worker is not None
AssertionError
By the way: urllib fetcher uses POST here (urlopen(full_url, body, timeout).)
Description
Motivation and Context
Have you tested this? If so, how?
I ran my jobs with this code and it works for me.