-
Notifications
You must be signed in to change notification settings - Fork 6k
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
octopus: mgr/dashboard: fix API tests to be py3 compatible #34759
Conversation
jenkins test dashboard backend |
backend API test failure:
|
@@ -935,7 +935,7 @@ def run_ceph_w(self, watch_channel=None): | |||
if watch_channel is not None: | |||
args.append("--watch-channel") | |||
args.append(watch_channel) | |||
proc = self.controller.run(args=args, wait=False, stdout=BytesIO()) | |||
proc = self.controller.run(args=args, wait=False, stdout=StringIO()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh, should add from six import StringIO
at the top of this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
six
.
because before using python3 for driving the octopus tests, we should be python2 compatible, but io.StringIO
in python2 expects unicode, while StringIO.StringIO
in python2 expects str. i don't want to investigate if any of vstart_runner's consumer would be annoyed at seeing unicode on Python2, so a safe bet would be six.StringIO
which is an alias of StringIO.StringIO
on python2 and io.StringIO
on python3. but if you are sure about this, io.StringIO
is the way to go.
but we don't have this issue when it comes to BytesIO
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed explanation, @tchaikov!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline with @tchaikov I added the missing import from six import StringIO
- I added it to the backport commit which uses StringIO
with an explanation why this change can not be cherry-picked from master.
jenkins test dashboard backend |
f94f4c3
to
1305f69
Compare
jenkins test dashboard backend |
* test_perf_counters_mgr_get * test_selftest_cluster_log Fixes: https://tracker.ceph.com/issues/45170 Fixes: https://tracker.ceph.com/issues/45246 Signed-off-by: Alfonso Martínez <almartin@redhat.com> (cherry picked from commit 0ac2964)
to be consistent with 8bfe977 Fixes: https://tracker.ceph.com/issues/45246 Signed-off-by: Kefu Chai <kchai@redhat.com> (cherry picked from commit 2de78c3) qa/tasks: Add missing StringIO import This is directly fixed in octopus since it's needed to get the dashboard backend API tests running based on the previous commits of this PR. There is a fix in master but this one uses "from io import StringIO" while we still need to be python2 compatible in octopus. So this import is done with "six" (expects str in python2 instead of "io" (expects unicode in python2). This import line is not cherry-picked from master due to the above mentioned reasons. Signed-off-by: Laura Paduano <lpaduano@suse.com>
1305f69
to
b5194ac
Compare
jenkins test dashboard backend |
jenkins test dashboard |
backport trackers:
backport of #34752
parent trackers:
this backport was staged using ceph-backport.sh version 15.1.1.389
find the latest version at https://github.com/ceph/ceph/blob/master/src/script/ceph-backport.sh