Enable the user change the setup of the cluster check process #836
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: runhouse/servers/cluster_servlet.py
Did you find this useful? React with a 👍 or 👎 |
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| @@ -1645,3 +1658,44 @@ def _check_for_child_configs(cls, config: dict): | |||
| config["default_env"] = default_env | |||
|
|
|||
| return config | |||
|
|
|||
| def _is_den_authed_and_saved_to_den(self, change_type: str): | |||
There was a problem hiding this comment.
wondering if this check is necessary, is there any harm in just updating the config with the interval field regardless if status check is explicitly enabled or disabled?
There was a problem hiding this comment.
@jlewitt1 Removed the check if the cluster is saved into den. If it is not, the post request will raise an error, which we catch and send an error log, explaining that the cluster is not saved and therefore we can't post its status.
If left the check of den_auth == True because:
- It is not pinging den, therefore it is a "cheap" check
- I think it is in our interests to clarify to the user that this is a feature requires Den auth / Den user. wdyt?
| if interval_size == -1: | ||
| if is_config_updated: |
There was a problem hiding this comment.
maybe we should add a log here anytime the config is updated, not just when its turned off
There was a problem hiding this comment.
@jlewitt1 This log is added as part of aupdate_status_check_interval_in_cluster_config method
BelSasha
force-pushed
the
sb/enbale-or-disable-status-polling
branch
from
8f27a55
to
66095b0
Compare
BelSasha
force-pushed
the
sb/enbale-or-disable-status-polling
branch
from
66095b0
to
91d0286
Compare
| """ | ||
| Stopping consecutively sending status to Den. | ||
| """ | ||
| if not self.den_auth: |
There was a problem hiding this comment.
what happens if i have a cluster with den auth enabled, then i decide to disable it. Will the status check still continue in this case?
There was a problem hiding this comment.
@jlewitt1 yeah good point, I updated the logic so the status check will stop in that case.
BelSasha
force-pushed
the
sb/enbale-or-disable-status-polling
branch
from
91d0286
to
0903dca
Compare
No description provided.