Feature: Test Orchestrator (astrolabe) #2
Conversation
prashantmital
force-pushed
the
feature/Test-Orchestrator
branch
from
1b5f6c8 to
b68df26
Compare
This comment has been minimized.
This comment has been minimized.
prashantmital
force-pushed
the
feature/Test-Orchestrator
branch
2 times, most recently
from
f6c46ab to
de9bece
Compare
prashantmital
changed the title
prashantmital
force-pushed
the
feature/Test-Orchestrator
branch
from
bd270c9 to
aefe8b6
Compare
setup.py
Outdated
| @@ -40,6 +40,7 @@ | |||
| 'pymongo>=3.10,<4', | |||
| 'dnspython>=1.16,<2', | |||
| 'pyyaml>=5,<6', | |||
| 'tabulate>=0.8,<0.9', | |||
There was a problem hiding this comment.
What feature does this library provide? Do we need it?
There was a problem hiding this comment.
It provides a quick way to convert a list of lists into a textual table of various styles. In our case, we want the rst format to be able to include the resulting table into docs. While the docs part is not yet done, take a look at the docgen.py file for a closer look.
To see it in action install the package and do astrolabe info environment-variables or astrolabe info default-values.
|
Sorry for the epic code churn in that last commit @ShaneHarvey. The next time you take a look, things should be a lot easier to understand. |
prashantmital
force-pushed
the
feature/Test-Orchestrator
branch
from
021687c to
7cb5d82
Compare
prashantmital
changed the title
astrolabe/spec_runner.py
Outdated
| driver_workload = json.dumps(self.spec.driverWorkload) | ||
| worker_subprocess = subprocess.Popen([ | ||
| sys.executable, self.config.workload_executor, connection_string, | ||
| driver_workload], stdout=subprocess.PIPE, stderr=subprocess.PIPE) |
There was a problem hiding this comment.
Since we PIPE the output, we need to continuously read the worker_subprocess output (via worker_subprocess.stdout.read(..)) or else we risk blocking the child process when the pipe fills up.
See the note in subprocess.wait:
This will deadlock when using stdout=PIPE or stderr=PIPE and the child process generates enough output to a pipe such that it blocks waiting for the OS pipe buffer to accept more data. Use Popen.communicate() when using pipes to avoid that.
https://docs.python.org/3/library/subprocess.html#subprocess.Popen.wait
A simpler option might be to redirect the worker_subprocess output to a file. Then we can open and read the file after the process exits.
There was a problem hiding this comment.
Just to be clear: there's no risk of deadlock in the current approach. The risk is that the child process can fill up the pipe with output and then it will block until we call worker_subprocess.communicate() down below.
There was a problem hiding this comment.
Great point. I have opened #9 for this.
astrolabe/cli.py
Outdated
| help='Salt for generating unique hashes.') | ||
|
|
||
| XUNITOUTPUT_OPTION = click.option( | ||
| '--xunit-output', type=click.STRING, default="xunit-output", |
There was a problem hiding this comment.
why are some of these OPTNAMES.XXX and others inline strings?
There was a problem hiding this comment.
The ones which can be displayed/tabulated using astrolabe info <...> are the ones that are specified using a OPTNAMES.XXX. Options with inline strings are not tabulated in the same manner and are not required to be (in my estimation).
astrolabe/spec_runner.py
Outdated
| coll = client.get_database( | ||
| self.spec.driverWorkload.database).get_collection( | ||
| self.spec.driverWorkload.collection) | ||
| coll.drop() |
There was a problem hiding this comment.
do you need to potentially catch ns not found? or is that not a problem for python?
There was a problem hiding this comment.
PyMongo ignores this error on drop() so no special handling is required.
astrolabe/spec_runner.py
Outdated
| cluster_config = final_config.clusterConfiguration | ||
| process_args = final_config.processArgs | ||
|
|
||
| if not cluster_config and not process_args: |
There was a problem hiding this comment.
should we consider doing validation much earlier, so you can bail before any I/O if need be?
There was a problem hiding this comment.
Good point. In general, we just need a lot more validation of the test specification. I have opened #10 to do this.
prashantmital
force-pushed
the
feature/Test-Orchestrator
branch
from
f50b38e to
f9d738f
Compare
Still needs:
mongod/mongos log retrieval functionality (wait until Atlas logs are current for driver ops; download them in a folder hierarchy that makes it easy to find the logs corresponding to a specific test; zip this file for upload to S3 after EVG run)(moved to follow-on work, see Add post test log retrieval functionality #4)loggingmodule (this includes INFO level logs instead of the current print statements that make it easier to follow the progress of a test run)