Bench web/utils #4
Conversation
Reviewer's Guide by SourceryThis pull request adds core infrastructure and benchmarking code for the BenchWeb project, including Docker configurations, database setup, test frameworks, and utilities for running benchmarks. Class diagram for Results and DockerHelper classesclassDiagram
class Results {
-Benchmarker benchmarker
-Config config
-String directory
-String file
-String uuid
-String name
-String environmentDescription
-Dict git
-int startTime
-int completionTime
-List concurrencyLevels
-List pipelineConcurrencyLevels
-List queryIntervals
-List cachedQueryIntervals
-List frameworks
-int duration
-Dict rawData
-Dict completed
-Dict succeeded
-Dict failed
-Dict verify
+parse(tests)
+parse_test(framework_test, test_type)
+parse_all(framework_test)
+write_intermediate(test_name, status_message)
+set_completion_time()
+upload()
+load()
+get_docker_stats_file(test_name, test_type)
+get_raw_file(test_name, test_type)
+get_stats_file(test_name, test_type)
+report_verify_results(framework_test, test_type, result)
+report_benchmark_results(framework_test, test_type, results)
+finish()
}
class DockerHelper {
-Benchmarker benchmarker
-DockerClient client
-DockerClient server
-DockerClient database
+clean()
+build(test, build_log_dir)
+run(test, run_log_dir)
+stop(containers)
+build_databases()
+start_database(database)
+build_wrk()
+test_client_connection(url)
+server_container_exists(container_id_or_name)
+benchmark(script, variables)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @gitworkflows - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding more robust error handling and validation in shell scripts, particularly around environment variables and command execution
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| problems.append(('fail', "`%s` should be `%s`" % | ||
| (''.join(current_neg), | ||
| ''.join(current_pos)), url)) | ||
| except: |
There was a problem hiding this comment.
issue (bug_risk): Broad exception handling swallows all errors silently
Consider logging the caught exception to aid debugging. Catching all exceptions silently makes it difficult to diagnose issues.
| else: | ||
| self.timestamp = time.strftime("%Y%m%d%H%M%S", time.localtime()) | ||
|
|
||
| self.run_test_timeout_seconds = 7200 |
There was a problem hiding this comment.
suggestion: Hardcoded timeout value should be configurable
Consider making the test timeout configurable through command line arguments or configuration file.
self.run_test_timeout_seconds = int(os.getenv('TEST_TIMEOUT_SECONDS', '7200'))| 6. Fix this `README.md` and open a pull request | ||
|
|
||
| Starting on line 49 is your actual `README.md` that will sit with your test implementation. Update all the dummy values to their correct values so that when people visit your test in our Github repository, they will be greated with information on how your test implementation works and where to look for useful source code. | ||
|
|
||
| After you have the real `README.md` file in place, delete everything above line 59 and you are ready to open a pull request. |
There was a problem hiding this comment.
nitpick (documentation): There's a typo: 'greated' should be 'greeted'
| with open(self.file, "w") as f: | ||
| f.write(json.dumps(self.__to_jsonable(), indent=2)) | ||
|
|
||
| def parse_test(self, framework_test, test_type): |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the parse_test() method to separate parsing logic from file I/O operations.
The parse_test() method has become difficult to maintain due to deep nesting and mixed responsibilities. Consider extracting the parsing logic into a helper method:
def parse_test(self, framework_test, test_type):
results = {'results': []}
stats = []
raw_file = self.get_raw_file(framework_test.name, test_type)
if os.path.exists(raw_file):
with open(raw_file) as raw_data:
results['results'] = self._parse_raw_data(raw_data)
# Process stats for valid results
for result in results['results']:
if 'startTime' in result and 'endTime' in result:
test_stats = self.__parse_stats(
framework_test, test_type,
result["startTime"], result["endTime"], 1)
stats.append(test_stats)
# Write stats file
with open(self.get_stats_file(framework_test.name, test_type) + ".json", "w") as f:
json.dump(stats, f, indent=2)
return results
def _parse_raw_data(self, raw_data):
"""Parse raw benchmark output data into result dictionaries"""
results = []
is_warmup = True
current_result = None
for line in raw_data:
if "Queries:" in line or "Concurrency:" in line:
is_warmup = False
current_result = None
continue
if "Warmup" in line or "Primer" in line:
is_warmup = True
continue
if not is_warmup:
if current_result is None:
current_result = {}
results.append(current_result)
self._parse_metric_line(line, current_result)
return resultsThis refactoring:
- Extracts core parsing logic into
_parse_raw_data() - Reduces nesting depth
- Makes the code more testable and maintainable
- Preserves all existing functionality
The helper method handles the line-by-line parsing while the main method focuses on file I/O and stats processing.
| import time | ||
|
|
||
|
|
||
| class BenchmarkConfig: |
There was a problem hiding this comment.
issue (complexity): Consider splitting the BenchmarkConfig class into smaller, focused components for network and path configuration.
The configuration class could be simplified by extracting Docker networking and path management into separate components. This would improve maintainability while keeping the core benchmark configuration focused. Here's a suggested refactoring:
class DockerNetworkConfig:
def __init__(self, network_mode, server_host, database_host, client_host):
self.network_mode = network_mode
if network_mode is None:
self.network = 'bw'
self.server_docker_host = "unix://var/run/docker.sock"
self.database_docker_host = "unix://var/run/docker.sock"
self.client_docker_host = "unix://var/run/docker.sock"
else:
self.network = None
self.server_docker_host = f"tcp://{server_host}:2375"
self.database_docker_host = f"tcp://{database_host}:2375"
self.client_docker_host = f"tcp://{client_host}:2375"
class BenchmarkPaths:
def __init__(self, fw_root):
self.fw_root = fw_root
self.db_root = os.path.join(fw_root, "infrastructure", "docker", "databases")
self.lang_root = os.path.join(fw_root, "frameworks")
self.results_root = os.path.join(fw_root, "results")
self.wrk_root = os.path.join(fw_root, "benchmarks", "load-testing", "wrk")
self.scaffold_root = os.path.join(fw_root, "benchmarks", "pre-benchmarks")
class BenchmarkConfig:
def __init__(self, args):
self._init_test_types(args.type)
self._copy_benchmark_args(args)
self.paths = BenchmarkPaths(os.getenv('FWROOT'))
self.network_config = DockerNetworkConfig(
args.network_mode,
args.server_host,
args.database_host,
args.client_host
)This separates concerns while maintaining functionality:
- DockerNetworkConfig handles network setup logic
- BenchmarkPaths manages path configuration
- BenchmarkConfig focuses on core benchmark settings
The main class becomes cleaner and more maintainable, while related configuration stays logically grouped.
| self.completed = dict() | ||
| self.succeeded = dict() | ||
| self.failed = dict() | ||
| self.verify = dict() |
There was a problem hiding this comment.
suggestion (code-quality): Replace dict() with {} (dict-literal)
| self.verify = dict() | |
| self.verify = {} |
Explanation
The most concise and Pythonic way to create a dictionary is to use the{}notation.
This fits in with the way we create dictionaries with items, saving a bit of
mental energy that might be taken up with thinking about two different ways of
creating dicts.
x = {"first": "thing"}Doing things this way has the added advantage of being a nice little performance
improvement.
Here are the timings before and after the change:
$ python3 -m timeit "x = dict()"
5000000 loops, best of 5: 69.8 nsec per loop
$ python3 -m timeit "x = {}"
20000000 loops, best of 5: 29.4 nsec per loop
Similar reasoning and performance results hold for replacing list() with [].
| self.failed = dict() | ||
| self.verify = dict() | ||
| for type in test_types: | ||
| self.rawData[type] = dict() |
There was a problem hiding this comment.
suggestion (code-quality): Replace dict() with {} (dict-literal)
| self.rawData[type] = dict() | |
| self.rawData[type] = {} |
Explanation
The most concise and Pythonic way to create a dictionary is to use the{}notation.
This fits in with the way we create dictionaries with items, saving a bit of
mental energy that might be taken up with thinking about two different ways of
creating dicts.
x = {"first": "thing"}Doing things this way has the added advantage of being a nice little performance
improvement.
Here are the timings before and after the change:
$ python3 -m timeit "x = dict()"
5000000 loops, best of 5: 69.8 nsec per loop
$ python3 -m timeit "x = {}"
20000000 loops, best of 5: 29.4 nsec per loop
Similar reasoning and performance results hold for replacing list() with [].
| ''' | ||
| Parses the given test and test_type from the raw_file. | ||
| ''' | ||
| results = dict() |
There was a problem hiding this comment.
suggestion (code-quality): Replace dict() with {} (dict-literal)
| results = dict() | |
| results = {} |
Explanation
The most concise and Pythonic way to create a dictionary is to use the{}notation.
This fits in with the way we create dictionaries with items, saving a bit of
mental energy that might be taken up with thinking about two different ways of
creating dicts.
x = {"first": "thing"}Doing things this way has the added advantage of being a nice little performance
improvement.
Here are the timings before and after the change:
$ python3 -m timeit "x = dict()"
5000000 loops, best of 5: 69.8 nsec per loop
$ python3 -m timeit "x = {}"
20000000 loops, best of 5: 29.4 nsec per loop
Similar reasoning and performance results hold for replacing list() with [].
| continue | ||
| if not is_warmup: | ||
| if rawData is None: | ||
| rawData = dict() |
There was a problem hiding this comment.
suggestion (code-quality): Replace dict() with {} (dict-literal)
| rawData = dict() | |
| rawData = {} |
Explanation
The most concise and Pythonic way to create a dictionary is to use the{}notation.
This fits in with the way we create dictionaries with items, saving a bit of
mental energy that might be taken up with thinking about two different ways of
creating dicts.
x = {"first": "thing"}Doing things this way has the added advantage of being a nice little performance
improvement.
Here are the timings before and after the change:
$ python3 -m timeit "x = dict()"
5000000 loops, best of 5: 69.8 nsec per loop
$ python3 -m timeit "x = {}"
20000000 loops, best of 5: 29.4 nsec per loop
Similar reasoning and performance results hold for replacing list() with [].
| the parent process' memory from the child process | ||
| ''' | ||
| if framework_test.name not in self.verify.keys(): | ||
| self.verify[framework_test.name] = dict() |
There was a problem hiding this comment.
suggestion (code-quality): Replace dict() with {} (dict-literal)
| self.verify[framework_test.name] = dict() | |
| self.verify[framework_test.name] = {} |
Explanation
The most concise and Pythonic way to create a dictionary is to use the{}notation.
This fits in with the way we create dictionaries with items, saving a bit of
mental energy that might be taken up with thinking about two different ways of
creating dicts.
x = {"first": "thing"}Doing things this way has the added advantage of being a nice little performance
improvement.
Here are the timings before and after the change:
$ python3 -m timeit "x = dict()"
5000000 loops, best of 5: 69.8 nsec per loop
$ python3 -m timeit "x = {}"
20000000 loops, best of 5: 29.4 nsec per loop
Similar reasoning and performance results hold for replacing list() with [].
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
PR Type
enhancement, configuration changes, documentation
Description
Results,Metadata,DockerHelper,Scaffolding,Benchmarker,FortuneHTMLParser,FrameworkTest,TimeLogger,BenchmarkConfig,AbstractTestType,TestTypefor various test types,PopenTimeout,Audit, and database operations for PostgreSQL and MongoDB.wrk, including pipeline, concurrency, and query scripts.Changes walkthrough 📝
13 files
wrk.dockerfile
Add Dockerfile for benchmarking with wrk on Ubuntu 24.04benchmarks/load-testing/wrk/wrk.dockerfile
postgres.dockerfile
Add Dockerfile for PostgreSQL 17 with configurationsinfrastructure/docker/databases/postgres/postgres.dockerfile
configurations.
mysql.dockerfile
Add Dockerfile for MySQL 9.0 with configurationsinfrastructure/docker/databases/mysql/mysql.dockerfile
mongodb.dockerfile
Add Dockerfile for MongoDB 7.0 with initializationinfrastructure/docker/databases/mongodb/mongodb.dockerfile
.siegerc
Add Configuration File for Siege Load Testing Toolinfrastructure/docker/databases/.siegerc
Dockerfile
Add Dockerfile for BW Docker Image Buildinfrastructure/docker/Dockerfile
dependencies.
my.cnf
Add MySQL Configuration File with Performance Settingsinfrastructure/docker/databases/mysql/my.cnf
postgresql.conf
Add PostgreSQL Configuration File with Performance Tuninginfrastructure/docker/databases/postgres/postgresql.conf
bw.service
Add Systemd Service File for BW Service Managementinfrastructure/docker/services/bw.service
Vagrantfile
Add Vagrantfile for Vagrant Environment Configurationinfrastructure/vagrant/Vagrantfile
benchmark_config.json
Add JSON Configuration Template for Benchmark Testsbenchmarks/pre-benchmarks/benchmark_config.json
60-postgresql-shm.conf
Add Shared Memory Configuration for PostgreSQLinfrastructure/docker/databases/postgres/60-postgresql-shm.conf
60-database-shm.conf
Add Shared Memory Configuration for MySQLinfrastructure/docker/databases/mysql/60-database-shm.conf
39 files
results.py
Implement Results class for benchmark data managementutils/results.py
Resultsclass for handling benchmark results.verifications.py
Add verification functions for HTTP responses and database updatesbenchmarks/verifications.py
metadata.py
Implement Metadata class for test configuration managementutils/metadata.py
Metadataclass for managing test metadata.docker_helper.py
Implement DockerHelper class for Docker container managementutils/docker_helper.py
DockerHelperclass for managing Docker operations.scaffolding.py
Implement Scaffolding class for new test setuputils/scaffolding.py
Scaffoldingclass for setting up new test environments.benchmarker.py
Implement Benchmarker class for orchestrating benchmark testsbenchmarks/benchmarker.py
Benchmarkerclass for orchestrating benchmark tests.fortune_html_parser.py
Implement FortuneHTMLParser class for HTML validationbenchmarks/fortune/fortune_html_parser.py
FortuneHTMLParserclass for parsing and validating HTML.framework_test.py
Implement FrameworkTest class for managing testsbenchmarks/framework_test.py
FrameworkTestclass for managing individual tests.time_logger.py
Implement TimeLogger class for execution time trackingutils/time_logger.py
TimeLoggerclass for tracking execution times.benchmark_config.py
Implement BenchmarkConfig class for benchmark settingsutils/benchmark_config.py
BenchmarkConfigclass for configuring benchmarksettings.
abstract_test_type.py
Add Abstract Base Class for Benchmark Test Typesbenchmarks/abstract_test_type.py
AbstractTestTypeclass as an abstract base class for testtypes.
requests.
logic.
fortune.py
Implement Fortune Test Type with HTML Parsingbenchmarks/fortune/fortune.py
TestTypeclass for the "fortune" test type.FortuneHTMLParser.abstract_database.py
Define Abstract Base Class for Database Operationsinfrastructure/docker/databases/abstract_database.py
AbstractDatabaseclass as an abstract base for databaseoperations.
management.
db.py
Implement Database Test Type with JSON Verificationbenchmarks/db/db.py
TestTypeclass for the "db" test type.output_helper.py
Add Logging Utilities with Color and Quiet Modeutils/output_helper.py
logfunction for logging with optional color and prefix.QuietOutputStreamclass for conditional output suppression.postgres.py
Implement PostgreSQL Database Operations and Logginginfrastructure/docker/databases/postgres/postgres.py
Databaseclass for PostgreSQL operations.mongodb.py
Implement MongoDB Database Operations and Logginginfrastructure/docker/databases/mongodb/mongodb.py
Databaseclass for MongoDB operations.plaintext.py
Implement Plaintext Test Type with Response Verificationbenchmarks/plaintext/plaintext.py
TestTypeclass for the "plaintext" test type.responses.
cached-query.py
Implement Cached Query Test Type with JSON Validationbenchmarks/cached-query/cached-query.py
TestTypeclass for the "cached-query" test type.query.py
Implement Query Test Type with JSON Validationbenchmarks/query/query.py
TestTypeclass for the "query" test type.update.py
Implement Update Test Type with JSON Validationbenchmarks/update/update.py
TestTypeclass for the "update" test type.json.py
Implement JSON Test Type with Response Verificationbenchmarks/json/json.py
TestTypeclass for the "json" test type.popen.py
Add Subprocess Management with Timeout Handlingutils/popen.py
PopenTimeoutclass for subprocess management with timeout.__init__.py
Implement Dynamic Database Module Loading and Validationinfrastructure/docker/databases/init.py
audit.py
Add Framework Audit Class for Consistency Checksutils/audit.py
Auditclass for auditing framework consistency.__init__.py
Implement Dynamic Test Type Module Loadingbenchmarks/init.py
create.js
Add MongoDB Initialization Script for Collectionsinfrastructure/docker/databases/mongodb/create.js
worldandfortunecollections.
pipeline.sh
Add Load Testing Script for Pipeline Using wrkbenchmarks/load-testing/wrk/pipeline.sh
wrkwith pipeline.concurrency.sh
Add Load Testing Script for Concurrency Using wrkbenchmarks/load-testing/wrk/concurrency.sh
wrkwith concurrency.query.sh
Add Load Testing Script for Queries Using wrkbenchmarks/load-testing/wrk/query.sh
wrkwith queryparameters.
bw-startup.sh
Add Startup Script for BW Docker Service Initializationinfrastructure/docker/services/bw-startup.sh
service.
bootstrap.sh
Add Bootstrap Script for Vagrant VM Setupinfrastructure/vagrant/bootstrap.sh
bw-shutdown.sh
Add Shutdown Script for Docker Cleanup and Resource Managementinfrastructure/docker/services/bw-shutdown.sh
entry.sh
Add Entry Script for Docker Container Initializationinfrastructure/docker/entry.sh
config.sh
Add PostgreSQL Configuration Script for Settings Updateinfrastructure/docker/databases/postgres/config.sh
postgresql.conf.core.rb
Add Vagrant Provisioning Script for Provider Configurationinfrastructure/vagrant/core.rb
pipeline.lua
Add Lua Script for HTTP Request Pipelining in wrkbenchmarks/load-testing/wrk/pipeline.lua
wrkto handle HTTP request pipelining.create-postgres.sql
Add SQL Script for PostgreSQL Table Creation and Populationinfrastructure/docker/databases/postgres/create-postgres.sql
WorldandFortune.create.sql
Add SQL Script for MySQL Table Creation and Populationinfrastructure/docker/databases/mysql/create.sql
worldandfortune.1 files
custom_motd.sh
Add custom message of the day script for Vagrantinfrastructure/vagrant/custom_motd.sh
2 files
README.md
Add README for Vagrant Development Environment Setupinfrastructure/vagrant/README.md
environment.
README.md
Add README Template for New Benchmark Test Setupbenchmarks/pre-benchmarks/README.md