Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/382#discussion_r212387730
--- Diff: bigtop-packages/src/charm/spark/layer-spark/reactive/spark.py ---
@@ -236,7 +235,8 @@ def reconfigure_spark():
# Almost all config changes should trigger a reinstall... except when
# changing the bigtop repo version. Repo version changes require the
user
# to run an action, so we skip the reinstall in that case.
- if not is_state('config.changed.bigtop_version'):
+ if not is_state('config.changed.bigtop_version') \
+ and not is_state('config.changed.spark_enable_thriftserver'):
--- End diff --
I'm a bit unsettled with this. If someone changed multiple config options
with something like `juju config spark driver_memory=2048M
spark_enable_thriftserver=true`, we'd miss the driver_mem change.
What if we adjusted the `reconfigure_spark` function to be:
```
@when('spark.started', 'config.changed')
@when_not('config.changed.bigtop_version',
'config.changed.spark_enable_thriftserver')
def reconfigure_spark():
...
```
We could then remove the check for `config.changed.bigtop_version` in the
`reconfigure_spark` handler (it gets handled anyway with the
`check_repo_version` function a few lines down). Then we could adjust your
`start_thrift` function to be:
```
@when('spark.started', 'config.changed.spark_enable_thriftserver')
def start_thrift():
enable_thrift = hookenv.config()['spark_enable_thriftserver']
if enable_thrift:
check_call(['/usr/lib/spark/sbin/start-thriftserver.sh'])
else:
check_call(['/usr/lib/spark/sbin/stop-thriftserver.sh'])
```
---
