Skip to content

Commit

Permalink
Change Module's endpoint from propery to function
Browse files Browse the repository at this point in the history
This allows us to set external=True or False, in case we want a locally-callable
endpoint. Previously if the endpoint was only locally callable, we would only return
None. This is now also consistent with Cluster's endpoint().
  • Loading branch information
dongreenberg committed Jan 19, 2024
1 parent 7830739 commit e1fd06c
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
2 changes: 1 addition & 1 deletion runhouse/resources/hardware/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def config_for_rns(self):
return config

def endpoint(self, external=False):
"""Endpoint for the cluster's RPC server. If external is True, will only return the external url,
"""Endpoint for the cluster's Daemon server. If external is True, will only return the external url,
and will return None otherwise (e.g. if a tunnel is required). If external is False, will either return
the external url if it exists, or will set up the connection (based on connection_type) and return
the internal url (including the local connected port rather than the sever port). If cluster is not up,
Expand Down
30 changes: 20 additions & 10 deletions runhouse/resources/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"_pointers",
"_endpoint",
"endpoint",
"set_endpoint",
"_client",
"access_level",
"visibility",
Expand Down Expand Up @@ -126,7 +127,8 @@ def config_for_rns(self):
# modules change across Runhouse versions.
config["signature"] = self.signature

config["endpoint"] = self.endpoint
# Only save the endpoint if it's present in _endpoint or externally accessible
config["endpoint"] = self.endpoint(external=True)
return config

@classmethod
Expand Down Expand Up @@ -264,20 +266,28 @@ def method_signature(self, method, rich=False):

return signature_metadata

@property
def endpoint(self):
"""The endpoint of the module on the cluster. If the module is local, this will be None."""
# Only return an endpoint if it's external, local endpoints are not useful
def endpoint(self, external: bool = False):
"""The endpoint of the module on the cluster. Returns an endpoint if one was manually set (e.g. if loaded
down from a config). If not, request the endpoint from the Module's system.
Args:
external: If True and getting the endpoint from the system, only return an endpoint if it's externally
accessible (i.e. not on localhost, not connected through as ssh tunnel). If False, return the endpoint
even if it's not externally accessible.
"""
if self._endpoint:
return self._endpoint

if (
self._system
and hasattr(self._system, "endpoint")
and self._system.endpoint(external=True)
and self._system.endpoint(external=external)
):
return f"{self._system.endpoint(external=True)}/{self.name}"
return self._endpoint
return f"{self._system.endpoint(external=external)}/{self.name}"

return None

@endpoint.setter
def endpoint(self, new_endpoint: str):
def set_endpoint(self, new_endpoint: str):
self._endpoint = new_endpoint

def _client(self):
Expand Down

0 comments on commit e1fd06c

Please sign in to comment.