[CLI] add support to run app on a specific cluster #13894
Merged
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
I can run this with ``` python -m pytest tests/tests_clusters/test_cluster_lifecycle.py ``` test fails ofc because both the assertions don't work yet
supported commands: - lightning clusters create - lightning clusters list - lightning clusters delete
verified e2e via ``` python -m pytest tests/tests_clusters/test_cluster_lifecycle.py::test_cluster_list ```
I can't validate this just yet because I don't have the feature flag enabled in prod.
Co-authored-by: William Falcon <[email protected]>
we need this as a second step once BYOC compute clusters can be created - running apps on specific clusters
nicolai86
requested review from
awaelchli,
manskx,
hhsecond,
williamFalcon,
Borda,
carmocca,
justusschock,
kaushikb11 and
rohitgr7
as code owners
manskx
reviewed
Aug 2, 2022
manskx
reviewed
Aug 2, 2022
manskx
approved these changes
Aug 2, 2022
hhsecond
previously requested changes
Aug 3, 2022
There was a problem hiding this comment.
Thanks for the PR @nicolai86. Overall, the flow looks good
I have made few comments about some of the early design choices we made which conflicts with the design of this PR. We don't necessarily have to go with the original design but we need to come to a conscious consensus on how we should go about it
awaelchli
approved these changes
Aug 4, 2022
| @@ -121,6 +130,9 @@ def run(): | |||
| @run.command("app") | |||
| @click.argument("file", type=click.Path(exists=True)) | |||
| @click.option("--cloud", type=bool, default=False, is_flag=True) | |||
| @click.option( | |||
| "--cluster-id", type=str, default=None, help="Run Lightning App on a specific Lightning AI BYOC compute cluster" | |||
There was a problem hiding this comment.
How does the cluster id relate to the cluster name given in the cloud compute?
Also, @nicolai86 can you give us an example of such an ID? Is it human readable or just a long number?
nicolai86
dismissed
hhsecond’s stale review
addressed comments that are applicable
12 tasks
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.
Follow up PR for #13835.
This PR changes the run command to support a
--cluster-idflag when--cloudis provided.This way customers can run applications on their own BYOC compute clusters.
Verification