New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pybind/mgr/restful: use list to pass hooks to create a `Pecan` instance #15646

Merged
merged 4 commits into from Jun 15, 2017

Conversation

Projects
None yet
2 participants
@tchaikov
Contributor

tchaikov commented Jun 13, 2017

pecan 0.3.2 introduced the new way to pass the hooks to construct a
Pecan instance, but the "callable" builder for hooks is not compatible
with pecan < 0.3.2. and ubuntu trusty ships pecan 0.3.0, so we need to
do this the old way: pass a list of hooks instead of a callable.

see https://github.com/pecan/pecan/blame/0.3.2/pecan/core.py and
pecan/pecan@fb2cf16

Fixes: http://tracker.ceph.com/issues/20258
Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov requested a review from b-ranto Jun 13, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 13, 2017

@tchaikov can you run it through the rados test suite to see if it still works with pecan version in centos? The 0.3.2 source code suggests it should but you never know. Or even better, can you update the test case yaml file ( qa/suites/rados/rest/mgr-restful.yaml) to run on trusty (and xenial) as well and run it through the rados test case?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 13, 2017

@b-ranto sure, will do.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 13, 2017

@b-ranto the test failed for a different reason:

@tchaikov tchaikov requested a review from liewegas Jun 13, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 13, 2017

i will rerun the test with master tomorrow.

master failed with the same problem as the previous centos run:

2017-06-14T04:59:14.690 INFO:tasks.workunit.client.a.mira041.stdout:prefix mgr/restful/x url https://127.0.0.1:9999
2017-06-14T04:59:14.976 INFO:tasks.ceph.mgr.x.mira041.stderr:127.0.0.1 - - [14/Jun/2017 04:59:14] "POST /pool?wait=yes HTTP/1.1" 500 -
2017-06-14T04:59:14.983 INFO:tasks.ceph.mgr.x.mira041.stderr:Error on request:
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:Traceback (most recent call last):
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/werkzeug/serving.py", line 177, in run_wsgi
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:    execute(self.server.app)
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/werkzeug/serving.py", line 165, in execute
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:    application_iter = app(environ, start_response)
2017-06-14T04:59:14.984 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/pecan/middleware/recursive.py", line 56, in __call__
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    return self.application(environ, start_response)
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/pecan/core.py", line 570, in __call__
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    self.handle_request(req, resp)
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/pecan/core.py", line 508, in handle_request
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    result = controller(*args, **kwargs)
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib64/ceph/mgr/restful/decorators.py", line 33, in decorated
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    return f(*args, **kwargs)
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib64/ceph/mgr/restful/api/pool.py", line 100, in post
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    args = request.json
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/pecan/core.py", line 35, in __getattr__
2017-06-14T04:59:14.985 INFO:tasks.ceph.mgr.x.mira041.stderr:    return getattr(obj, attr)
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib/python2.7/site-packages/webob/request.py", line 701, in _json_body__get
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:    return json.loads(self.body.decode(self.charset))
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib64/python2.7/json/__init__.py", line 338, in loads
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:    return _default_decoder.decode(s)
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib64/python2.7/json/decoder.py", line 365, in decode
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:  File "/usr/lib64/python2.7/json/decoder.py", line 383, in raw_decode
2017-06-14T04:59:14.986 INFO:tasks.ceph.mgr.x.mira041.stderr:    raise ValueError("No JSON object could be decoded")
2017-06-14T04:59:14.987 INFO:tasks.ceph.mgr.x.mira041.stderr:ValueError: No JSON object could be decoded

tchaikov added some commits Jun 13, 2017

pybind/mgr/restful: use list to pass hooks to create a `Pecan` instance
pecan 0.3.2 introduced the new way to pass the hooks to construct a
Pecan instance, but the "callable" builder for hooks is not compatible
with pecan < 0.3.2. and ubuntu trusty ships pecan 0.3.0, so we need to
do this the old way: pass a list of hooks instead of a callable.

see https://github.com/pecan/pecan/blame/0.3.2/pecan/core.py and
pecan/pecan@fb2cf16

Fixes: http://tracker.ceph.com/issues/20258
Signed-off-by: Kefu Chai <kchai@redhat.com>
qa/workunits/rest/test_mgr_rest_api.py: set headers for requests
i think it's a bug in httplib shipped in old distro.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Revert "qa/suites/rados/rest/mgr-restful: test on centos"
This reverts commit be88220.
we can workaround the limit of old requests by using the backward
compatible way.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 15, 2017

@b-ranto @liewegas the tests pass now. could you take a look again?

args = request.json
except ValueError:
response.status = 400
return {'message': 'Bad request'}

This comment has been minimized.

@b-ranto

b-ranto Jun 15, 2017

Contributor

Can we be a bit more specific here and return something like 'Bad request: malformed json'?

pybind/mgr/restful: return 400 on bad request
we should return 500 on that case. it's the client's fault not feeding us
with expected requests.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 15, 2017

@b-ranto updated and repushed.

@b-ranto

lgtm

@tchaikov tchaikov merged commit 3ddbfcd into ceph:master Jun 15, 2017

2 of 4 checks passed

arm64 make check arm64 make check started
Details
make check running make check
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details

@tchaikov tchaikov deleted the tchaikov:wip-20258 branch Jun 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment