Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cluster default env #678

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Cluster default env #678

merged 2 commits into from
Apr 22, 2024

Conversation

carolineechen
Copy link
Collaborator

@carolineechen carolineechen commented Mar 28, 2024

default env flow

  • passed in at cluster initialization
  • default env installations is set up on cluster using pure ssh commands
  • run the http server start inside the env, passing in default_env which is propagated to the cluster env set up
  • put the default env resource on the cluster, and set the env vars after

behavior

  • cluster.run, cluster.put, etc runs inside cluster.default_env if not explicitly specified
  • if no default env is provided, defaults to base_env

TODOs (can be in follow up?)

  • env vars support -- currently set w/ after runhouse start on the default env. can be instead appended to runhouse start command and as a result flow into all subprocesses (flow to add to start cmd kinda annoying tho)

@carolineechen carolineechen force-pushed the cc/cluster-init-env branch 3 times, most recently from b033875 to 615e2ed Compare March 29, 2024 03:05
@carolineechen carolineechen changed the base branch from main to cc/sky-utils-autostop April 2, 2024 20:58
@carolineechen carolineechen changed the base branch from cc/sky-utils-autostop to cc/cmd-util April 2, 2024 21:19
@carolineechen carolineechen force-pushed the cc/cmd-util branch 2 times, most recently from a7ed140 to ab4625f Compare April 4, 2024 01:25
@carolineechen carolineechen force-pushed the cc/cmd-util branch 3 times, most recently from e204d8a to ec01315 Compare April 5, 2024 05:47
@carolineechen carolineechen changed the base branch from cc/cmd-util to cc/pkg-tests April 5, 2024 05:49
@carolineechen carolineechen force-pushed the cc/pkg-tests branch 2 times, most recently from 726f4d8 to 8d164d2 Compare April 7, 2024 00:24
@carolineechen carolineechen force-pushed the cc/pkg-tests branch 2 times, most recently from 3f399c8 to 5e0124b Compare April 9, 2024 02:39
@carolineechen carolineechen changed the base branch from cc/pkg-tests to cc/from-config-arg April 9, 2024 02:39
@@ -148,6 +148,9 @@ def _print_status(config):

first_info_to_print = ["den_auth", "server_connection_type", "server_port"]

if config.get("default_env") and isinstance(config["default_env"], Dict):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably be if config.get("default_env") is not None and ...

@@ -205,12 +206,10 @@ def to(
new_env.secrets = self._secrets_to(system)

if isinstance(system, Cluster):
if new_env.name == Env.DEFAULT_NAME:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused on this check

if self.is_up():
self.check_server()
if not self.get(self._default_env.name):
self._sync_default_env_to_cluster()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of doing sync and then put resource, could I not just do .to

Copy link
Contributor

rohinb2 commented Apr 22, 2024

Merge activity

  • Apr 22, 4:46 PM EDT: @rohinb2 started a stack merge that includes this pull request via Graphite.
  • Apr 22, 4:58 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 22, 4:59 PM EDT: @rohinb2 merged this pull request with Graphite.

@rohinb2 rohinb2 changed the base branch from cc/from-config-arg to main April 22, 2024 20:56
@rohinb2 rohinb2 merged commit dde13ea into main Apr 22, 2024
11 checks passed
@jlewitt1 jlewitt1 deleted the cc/cluster-init-env branch April 24, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants