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
[BEAM-7664] add more Python GBK Flink test cases #9106
[BEAM-7664] add more Python GBK Flink test cases #9106
Conversation
573c27f
to
2d85bcf
Compare
run seed job |
Run Load Tests Python GBK Flink Batch |
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! I left some suggestions but they're minor so I think we're almost there. :)
] | ||
]} | ||
|
||
def testConfigurationWithSixteenWorkers = { datasetName -> [ |
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.
I think we should try keep the configurations together in one list if possible. Then do something like (pseudocode, just to sketch the idea):
setup5WorkerFlink()
def testsToRun = testConfigurations.findAll { it -> it.jobProperties.parallelism = 5 }
run (testsToRun)
scaleCluster(16 /* workers */);
def testsToRun = testConfigurations.findAll { it -> it.jobProperties.parallelism = 16 }
run(testsToRun)
In case we have some more sophisticated criteria, we have all the data in one place and construct the query however we want. :)
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.
Ok, I changed it so it's more flexible thanks to few closures :)
|
||
loadTestsBuilder.loadTest(scope, testConfig.title, testConfig.runner, testConfig.sdk, testConfig.jobProperties, testConfig.itClass) | ||
numberOfWorkers = 5 | ||
infra.scaleCluster(scope, jenkinsJobName, numberOfWorkers) |
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.
This scaling sounds really cool. 😎
5c3b76a
to
1ec3008
Compare
run seed job |
Run Load Tests Python GBK Flink Batch |
@lgajowy This is ready, I applied your suggestion. I also rebased with master (there were some conflicts) |
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.
Code looks ok. Could you reorganize the branch history and squash commits that are left after review? I will merge after that. Thanks!
8899f77
to
dace04f
Compare
Thank you @lgajowy I just reorganised it. |
Run seed job |
Please add a meaningful description for your change here
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.