Add uses_bulkdata argument to paasta spark run instance_config #4005
Conversation
This makes the change to paasta spark run so that https://github.yelpcorp.com/sysgit/yelpsoa-configs/pull/52010 will work as expected. This works by adding the uses_bulkdata key to the intsance config if the spark job has the key present and set to true. I have added this arg to the tests so that they pass, and also created a test so that we can check all the different ways that uses_bulkdata can be set, either on paasta spark-run as an argument, or in the instance config. See #3995 for more information about why we're doing this.
SuperMatt
force-pushed
the
u/mames/PERES-5194-uses-bulkdata-spark-run
branch
from
7c084d3 to
23fcbfa
Compare
| assert ( | ||
| mock_get_instance_config.return_value.config_dict["uses_bulkdata"] == expected | ||
| ) |
There was a problem hiding this comment.
we've mocked this on L1436 - so this assertion isn't doing much atm (I guess right now this test is really just checking that spark_run.paasta_spark_run(args) doesn't crash if we remove this always-passing assertion?)
imo, deleting this test is fine (the new logic is pretty trivial) or updating the test to assert that the correct mounts are in place in the final command would be where we should go from here :)
|
Discussing with @nemacysts, we think that we don't need a test that the volume is present because we're testing that uses_bulkdata ends up on the instance config, but then we have another test already for the volume being added when uses_bulkdata is set to true, which can be seen here. Adding a spark test for this volume being included would be redundent. |
This makes the change to paasta spark run so that https://github.yelpcorp.com/sysgit/yelpsoa-configs/pull/52010 will work as expected.
This works by adding the uses_bulkdata key to the intsance config if the spark job has the key present and set to true.
I have added this arg to the tests so that they pass, and also created a test so that we can check all the different ways that uses_bulkdata can be set, either on paasta spark-run as an argument, or in the instance config.
See #3995 for more information about why we're doing this.