Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/382#discussion_r212650418
--- 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 --
@buggtb, danger! this isn't going to work and the mighty @johnsca has
confirmed the error of my ways. Since the handler is a catch-all with
'config.changed', if something is set in addition to `spark_enable_thrift`,
we'll skip the handler because of the @when_not -- basically forget my
suggestion to `reconfigure_spark`. We'll need to keep what you had to process
config_changed, skipping the reinstall if our bigtop_version or
spark_enable_thrift are set.
Still, I think the `start_thrift` method would nice to refactor to run
@when(spark.started, spark_thrift_enabled) and drop the data_changed logic.
---
