You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Marton Greber (Code Review)" <ge...@cloudera.org> on 2023/04/03 08:21:46 UTC
[kudu-CR] [Python] Support setting startup flags in tests
Marton Greber has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19679
Change subject: [Python] Support setting startup flags in tests
......................................................................
[Python] Support setting startup flags in tests
Currently it is not streamlined, how to set Kudu master, tserver startup
flags in Python tests. This patch adds master_extra_startup_flags() and
tserver_extra_startup_flags() decorators to KuduTestBase in order to
tackle this issue.
The decorators just set the class variables decorator_extra_master_flags
and decorator_extra_tserver_flags. During cluster setup, these are
appended to other default flags, which are hard coded into the Kudu
Python test framework. In the tearDownClass() function, the above lists
need to be wiped, else flags are spilling over from previous test
classes.
In test_common.py a basic test is added, which just starts up and shuts
down a test cluster, without throwing any errors. This is just to check
basic turnaround. If a test developer defines an unknown master flag for
example, the following type of message is raised:
"Exception: Error in response: {'code': 'RUNTIME_ERROR', 'message':
'failed to start masters ... "
In a given test run, after an unsuccessful cluster setup (above line),
chrony is not properly disposed of, and a follow-up test will time out
with error "Another chronyd may already be running". This is blocks
adding negative tests for this patch. I created a jira to track this
issue: KUDU-3464
Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
---
M python/kudu/tests/common.py
M python/kudu/tests/test_common.py
2 files changed, 74 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/19679/1
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 1
Gerrit-Owner: Marton Greber <gr...@gmail.com>
[kudu-CR] [Python] Support setting startup flags in tests
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19679
to look at the new patch set (#2).
Change subject: [Python] Support setting startup flags in tests
......................................................................
[Python] Support setting startup flags in tests
Currently it is not streamlined, how to set Kudu master, tserver startup
flags in Python tests. This patch adds master_extra_startup_flags() and
tserver_extra_startup_flags() decorators to KuduTestBase in order to
tackle this issue.
The decorators just set the class variables decorator_extra_master_flags
and decorator_extra_tserver_flags. During cluster setup, these are
appended to other default flags, which are hard coded into the Kudu
Python test framework. In the tearDownClass() function, the above lists
need to be wiped, else flags are spilling over from previous test
classes.
In test_common.py a basic test is added, which just starts up and shuts
down a test cluster, without throwing any errors. This is just to check
basic turnaround. If a test developer defines an unknown master flag for
example, the following type of message is raised:
"Exception: Error in response: {'code': 'RUNTIME_ERROR', 'message':
'failed to start masters ... "
In a given test run, after an unsuccessful cluster setup (above line),
chrony is not properly disposed of, and a follow-up test will time out
with error "Another chronyd may already be running". This is blocks
adding negative tests for this patch. I created a jira to track this
issue: KUDU-3464
Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
---
M python/kudu/tests/common.py
M python/kudu/tests/test_common.py
2 files changed, 74 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/19679/2
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 2
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [Python] Support setting startup flags in tests
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Chovan, Alexey Serbin, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19679
to look at the new patch set (#3).
Change subject: [Python] Support setting startup flags in tests
......................................................................
[Python] Support setting startup flags in tests
Currently it is not streamlined, how to set Kudu master, tserver startup
flags in Python tests. This patch adds master_extra_startup_flags() and
tserver_extra_startup_flags() decorators to KuduTestBase in order to
tackle this issue.
The decorators just set the class variables decorator_extra_master_flags
and decorator_extra_tserver_flags. During cluster setup, these are
appended to other default flags, which are hard coded into the Kudu
Python test framework. In the tearDownClass() function, the above lists
need to be wiped, else flags are spilling over from previous test
classes.
In test_common.py a basic test is added, which just starts up and shuts
down a test cluster, without throwing any errors. This is just to check
basic turnaround. If a test developer defines an unknown master flag for
example, the following type of message is raised:
"Exception: Error in response: {'code': 'RUNTIME_ERROR', 'message':
'failed to start masters ... "
In a given test run, after an unsuccessful cluster setup (above line),
chrony is not properly disposed of, and a follow-up test will time out
with error "Another chronyd may already be running". This is blocks
adding negative tests for this patch. I created a jira to track this
issue: KUDU-3464
Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
---
M python/kudu/tests/common.py
M python/kudu/tests/test_common.py
2 files changed, 74 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/19679/3
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 3
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] [Python] Support setting startup flags in tests
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Chovan, Alexey Serbin, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19679
to look at the new patch set (#4).
Change subject: [Python] Support setting startup flags in tests
......................................................................
[Python] Support setting startup flags in tests
Currently it is not streamlined, how to set Kudu master, tserver startup
flags in Python tests. This patch adds master_extra_startup_flags() and
tserver_extra_startup_flags() decorators to KuduTestBase in order to
tackle this issue.
The decorators just set the class variables decorator_extra_master_flags
and decorator_extra_tserver_flags. During cluster setup, these are
appended to other default flags, which are hard coded into the Kudu
Python test framework. In the tearDownClass() function, the above lists
need to be wiped, else flags are spilling over from previous test
classes.
In test_common.py a basic test is added, which just starts up and shuts
down a test cluster, without throwing any errors. This is just to check
basic turnaround. If a test developer defines an unknown master flag for
example, the following type of message is raised:
"Exception: Error in response: {'code': 'RUNTIME_ERROR', 'message':
'failed to start masters ... "
In a given test run, after an unsuccessful cluster setup (above line),
chrony is not properly disposed of, and a follow-up test will time out
with error "Another chronyd may already be running". This is blocks
adding negative tests for this patch. I created a jira to track this
issue: KUDU-3464
Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
---
M python/kudu/tests/common.py
M python/kudu/tests/test_common.py
2 files changed, 74 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/19679/4
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 4
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] [Python] Support setting startup flags in tests
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has abandoned this change. ( http://gerrit.cloudera.org:8080/19679 )
Change subject: [Python] Support setting startup flags in tests
......................................................................
Abandoned
As it turns out, it is best to upgrade the current test suite with a proper mini kudu like in the java client.
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 4
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] [Python] Support setting startup flags in tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19679 )
Change subject: [Python] Support setting startup flags in tests
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG@7
PS4, Line 7: setting startup flags
adding extra flags ?
http://gerrit.cloudera.org:8080/#/c/19679/4//COMMIT_MSG@28
PS4, Line 28: chrony
chronyd
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py
File python/kudu/tests/common.py:
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@35
PS4, Line 35: master services
masters
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@49
PS4, Line 49: tserver services
either 'tservers' or 'tablet servers'
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/common.py@130
PS4, Line 130: extra_tserver_flags
Typo? Should probably be decorator_extra_tserver_flags?
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/test_common.py
File python/kudu/tests/test_common.py:
http://gerrit.cloudera.org:8080/#/c/19679/4/python/kudu/tests/test_common.py@56
PS4, Line 56: @master_extra_startup_flags(extra_master_flags)
: @tserver_extra_startup_flags(extra_tserver_flags)
: class TestKuduTestBaseStartupFlagsPositive(KuduTestBase, CompatUnitTest):
:
: def setUp(cls):
: pass
:
: def test_startup_flags_positive(self):
: pass
It seems this test doesn't cover the newly added functionality: there is a typo at https://gerrit.cloudera.org/#/c/19679/4/python/kudu/tests/common.py@130 and this test didn't catch that.
I'd think that a proper test would check the corresponding processes have been actually started with the set of provided flags, no?
--
To view, visit http://gerrit.cloudera.org:8080/19679
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2423b29707e58d152aff934908fb3f0400b75da4
Gerrit-Change-Number: 19679
Gerrit-PatchSet: 4
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Apr 2023 05:57:06 +0000
Gerrit-HasComments: Yes