MLCOMPUTE-1160 | consolidate tron and paasta logic to service config lib #139
Conversation
| @@ -347,24 +347,6 @@ def _get_local_spark_env( | |||
| } | |||
|
|
|||
|
|
|||
| def _get_k8s_resource_name_limit_size_with_hash(name: str, limit: int = 63, suffix: int = 4) -> str: | |||
There was a problem hiding this comment.
Adding docs: Moved to utils.
| @@ -929,31 +911,44 @@ def _append_event_log_conf( | |||
|
|
|||
| if len(self.spark_srv_conf.items()) == 0: | |||
| log.warning('spark_srv_conf is empty, disable event log') | |||
| spark_opts.update({"spark.eventLog.enabled": "false"}) | |||
There was a problem hiding this comment.
spark.eventLog.enabled is default to false, perhaps we don't need this line if srv_config is empty?
There was a problem hiding this comment.
I have added this to be explicit and not leave the state to the default behaviour, since the log clearly says we are disabling it. I can remove this if you feel strongly against it
There was a problem hiding this comment.
I see, both looks okay to me
There was a problem hiding this comment.
Optional: Maybe we can remove this whole section? I can't think of a reason on why to disable to event log hen spark-srv-conf is empty. I recall generally spark event log used to be disabled to avoid risk of leaking of secrets. Since that is not happening anymore, AFAIK, maybe we can remove this section?
|
Did you mean to attach fluffy for the Jupyter config diff as an example? |
Testing:
Paasta:
Config diff between older version and this version: https://fluffy.yelpcorp.com/i/p4hQp4kN0lmsCJ7zLn3n0bRdm0W451Kz.html
Jupyter:
Config diff between older version and this version:
https://fluffy.yelpcorp.com/i/s27lb1H0sVbzb3q7cqjCrM3PbpkgPDKD.html