Skip to content
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

mgr/dashboard: pool: fix python3 dict_keys error #21636

Merged
merged 1 commit into from Apr 25, 2018

Conversation

rjfd
Copy link
Contributor

@rjfd rjfd commented Apr 25, 2018

The value returned by dict.keys() in python 3 is not of type list and is not JSON serializable.
Therefore we need to convert the result to a list.

Signed-off-by: Ricardo Dias rdias@suse.com

@rjfd rjfd added this to the mimic milestone Apr 25, 2018
@LenzGr LenzGr requested a review from p-se April 25, 2018 08:01
@@ -15,7 +15,7 @@ class Pool(RESTController):
@classmethod
def _serialize_pool(cls, pool, attrs):
if not attrs or not isinstance(attrs, list):
attrs = pool.keys()
attrs = list(pool.keys())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, pool.keys() returns an iterator in Python 3. But attrs is used only once in line 23. I'd think, this line should actually be OK, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, good catch!

Copy link
Contributor

@sebastian-philipp sebastian-philipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks sane.

@sebastian-philipp
Copy link
Contributor

May I say that I really dislike the use of iterators in Python 3? They introduced a ton of unnecessary bugs this way.

Signed-off-by: Ricardo Dias <rdias@suse.com>
@rjfd rjfd force-pushed the wip-dashboard-fix-pool-py3 branch from 6ec4f9b to f866072 Compare April 25, 2018 08:28
@rjfd
Copy link
Contributor Author

rjfd commented Apr 25, 2018

@sebastian-philipp addressed your comment, and pushed changes.

@p-se
Copy link
Contributor

p-se commented Apr 25, 2018

lgtm

@LenzGr LenzGr merged commit c783b74 into ceph:master Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants