Fix ping-and-up test for den launched clusters #1684
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BelSasha
changed the title
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
1597c5d to
c75846e
Compare
BelSasha
changed the title
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
5 times, most recently
from
e31d961 to
6524d80
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
2 times, most recently
from
c20cd82 to
4013fe4
Compare
BelSasha
commented
Jan 15, 2025
| cluster_den_status: list = read_resp_data(resp) | ||
|
|
||
| if not cluster_den_status: | ||
| logger.warning("Failed to update cluster info") |
There was a problem hiding this comment.
Decided to go with "Failed to update cluster info" since we are updating cluster info (such as compute properties etc), and not just cluster status.
@jlewitt1
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
2 times, most recently
from
0074691 to
97d564c
Compare
jlewitt1
reviewed
Jan 16, 2025
| cluster_dict = self._sky_status(refresh=not dryrun) | ||
| self._populate_connection_from_status_dict(cluster_dict) | ||
| if self.launcher == LauncherType.DEN: | ||
| cluster_uri = rns_client.format_rns_address(self.rns_address or self.name) |
There was a problem hiding this comment.
if we're using the den launcher it always have an rns address, is there a reason we have the or self.name here?
There was a problem hiding this comment.
Yeah, the first time we call self._update_from_status(dryrun=True) is during the cluster construction, before we save the cluster (relevant for both den and local launched clusters), therefore we don't have rns_address yet.
(Code reference)
There was a problem hiding this comment.
so in that case isn't it enough to rely on the RNS address, and if it's none not hit den?
There was a problem hiding this comment.
yeah, good point, agreed, pushed an update
jlewitt1
reviewed
Jan 21, 2025
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
97d564c to
ee1469a
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
2 times, most recently
from
6cbbf68 to
768ea7a
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
2 times, most recently
from
1edaa75 to
53ef430
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
from
1f9481c to
6c9cdd9
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
53ef430 to
a2f72d9
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
from
6c9cdd9 to
9573009
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
3 times, most recently
from
c0be80e to
6f580a5
Compare
BelSasha
force-pushed
the
handling-shared-cluster-creds
branch
from
6f580a5 to
814246b
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
14753f6 to
b82ab35
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
6 times, most recently
from
2280a44 to
6718847
Compare
BelSasha
force-pushed
the
handling-shared-cluster-creds
branch
from
6718847 to
8789a16
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
b82ab35 to
dd92066
Compare
jlewitt1
reviewed
Jan 26, 2025
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
dd92066 to
c5ef2e1
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
3 times, most recently
from
304b5ff to
0b66b12
Compare
BelSasha
force-pushed
the
handling-shared-cluster-creds
branch
from
0b66b12 to
65cf6d8
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
c5ef2e1 to
493b0d7
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
from
65cf6d8 to
20d6b9f
Compare
jlewitt1
force-pushed
the
handling-shared-cluster-creds
branch
4 times, most recently
from
74507f6 to
bac7432
Compare
BelSasha
force-pushed
the
sb/fix-ping-and-up-test
branch
from
493b0d7 to
c3c29d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixing the logic of
cluster._ping(retry=True)for den_launched clusters.