-
Notifications
You must be signed in to change notification settings - Fork 511
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
Readme cleanup, docs URL and CONTRIBUTING.md #1010
Changes from 2 commits
9037a5d
7b8f464
1ad4b6d
6545a04
4b39845
a02c775
fde88ff
daa7392
46709c8
00d10e5
ba12e37
8f88c88
675d6de
6e776ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
# Contributing to SkyPilot | ||
|
||
Thank you for your interest in contributing to SkyPilot! We welcome and value | ||
all contributions to the project, including but not limited to: | ||
|
||
* [Bug reports](https://github.com/sky-proj/sky/issues) and [discussions](https://github.com/sky-proj/sky/discussions) | ||
* [Pull requests](https://github.com/sky-proj/sky/pulls) for bug fixes and new features | ||
* Test cases to make the codebase more robust | ||
* Examples | ||
* Documentation | ||
* Tutorials, blog posts and talks on SkyPilot | ||
|
||
## Contributing Code | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bit of confusion between the bullet points mentioned in
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thinking maybe change the intro section to match the logical structure below. |
||
|
||
We use Github to track issues and features. For new contributors, we recommend looking at issues labeled ["good first issue"](https://github.com/sky-proj/sky/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22+). | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Installing SkyPilot for development | ||
We recommend using editable mode (`-e`) when installing: | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
```bash | ||
# SkyPilot requires python >= 3.6 and < 3.10. | ||
# You can just install the dependencies for | ||
# certain clouds, e.g., ".[aws,azure,gcp]" | ||
pip install -e ".[all]" | ||
pip install -r requirements-dev.txt | ||
``` | ||
IMPORTANT: Please `export SKYPILOT_DEV=1` before running the CLI commands in the terminal, so that developers' usage logs do not pollute the actual user logs. | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Environment variables for developers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proposal: how about we move this to the end of this doc? It's more important to read about things like Testing, style guide, etc. Section order that feels more natural to me (from here on):
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! I do want to keep testing before submitting pull requests, so the order now is:
|
||
- `export SKYPILOT_DEV=1` to send usage logs to dev space. | ||
- `export SKYPILOT_DISABLE_USAGE_COLLECTION=1` to disable usage logging. | ||
- `export SKYPILOT_DEBUG=1` to show debugging logs (use logging.DEBUG level). | ||
- `export SKYPILOT_MINIMIZE_LOGGING=1` to minimize the logging for demo purpose. | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Testing | ||
To run smoke tests: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, there is extra work needed to install Testing dependencies. Good to add that too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, those should be covered in |
||
``` | ||
bash tests/run_smoke_tests.sh | ||
|
||
# Run one of the smoke tests | ||
bash tests/run_smoke_tests.sh test_minimal | ||
``` | ||
|
||
For profiling code, use: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe delete. First time I have seen this profiling text on a Contribution md file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's generally useful to have these snippets here and the contributing guide can be verbose in the interest of completeness. Also okay to remove if you feel strongly otherwise |
||
``` | ||
pip install tuna # Tuna is used for visualization of profiling data. | ||
python3 -m cProfile -o sky.prof -m sky.cli status # Or some other command | ||
tuna sky.prof | ||
``` | ||
|
||
### Submitting pull requests | ||
- Fork the SkyPilot repository and create a new branch for your changes. | ||
- If relevant, add tests for your changes. For changes that touch the core system, run the [smoke tests](#testing) and ensure they pass. | ||
- Follow the [Google style guide](https://google.github.io/styleguide/pyguide.html). | ||
- After you commit, format your code with [`format.sh`](./format.sh). | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Push your changes to your fork and open a pull request in the SkyPilot repository. | ||
- In the PR description, write a `Tested:` section to describe relevant tests performed. | ||
|
||
### Dump timeline | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Timeline is useful for performance analysis and debugging in SkyPilot. | ||
|
||
Here are the APIs: | ||
|
||
```python | ||
from utils import timeline | ||
|
||
|
||
# record a function in the timeline with the function path name | ||
@timeline.event | ||
def f(): ... | ||
|
||
|
||
# record a function in the timeline using name='my_name' | ||
@timeline.event(name='event_name') | ||
def f(): ... | ||
|
||
|
||
# record an event over a code block in the timeline: | ||
with timeline.Event(name='event_name'): | ||
... | ||
|
||
# use a file lock with event: | ||
with timeline.FileLockEvent(lockpath): | ||
pass | ||
``` | ||
|
||
To dump the timeline, set environment variable `SKYPILOT_TIMELINE_FILE_PATH` to a file path. | ||
|
||
View the dumped timeline file using `Chrome` (chrome://tracing) or [Perfetto](https://ui.perfetto.dev/). | ||
|
||
|
||
### Some general engineering practice suggestions | ||
|
||
These are suggestions, not strict rules to follow. When in doubt, follow the [style guide](https://google.github.io/styleguide/pyguide.html). | ||
|
||
* Use `TODO(author_name)`/`FIXME(author_name)` instead of blank `TODO/FIXME`. This is critical for tracking down issues. You can write TODOs with your name and assign it to others (on github) if it is someone else's issue. | ||
* Delete your branch after merging it. This keeps the repo clean and faster to sync. | ||
* Use an exception if this is an error. Only use `assert` for debugging or proof-checking purposes. This is because exception messages usually contain more information. | ||
* Use modern python features and styles that increases code quality. | ||
* Use f-string instead of `.format()` for short expressions to increase readability. | ||
* Use `class MyClass:` instead of `class MyClass(object):`. The later one was a workaround for python2.x. | ||
* Use `abc` module for abstract classes to ensure all abstract methods are implemented. | ||
* Use python typing. But you should not import external objects just for typing. Instead, import typing-only external objects under `if typing.TYPE_CHECKING:`. | ||
|
||
### Updating the SkyPilot docker image | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
We maintain a docker image for users to easily run SkyPilot without requiring any installation. This image is manually updated with the following steps: | ||
|
||
1. Authenticate with SkyPilot ECR repository. Contact `romil.bhardwaj@berkeley.edu` for access: | ||
``` | ||
aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws/a9w6z7w5 | ||
``` | ||
|
||
2. Build and tag the docker image: | ||
``` | ||
docker build -t public.ecr.aws/a9w6z7w5/sky:latest . | ||
``` | ||
|
||
3. Push the image to ECR: | ||
``` | ||
docker push public.ecr.aws/a9w6z7w5/sky:latest | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,117 +1,67 @@ | ||
# SkyPilot | ||
|
||
![logo](docs/source/images/SkyPilot-logo-wide.png) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use some markdown magic to limit the width? Currently it feels too wide compared to size of the text. E.g., https://github.com/ray-project/ray/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! Set to |
||
|
||
![pytest](https://github.com/sky-proj/sky/actions/workflows/pytest.yml/badge.svg) | ||
|
||
SkyPilot is a framework to run any workload seamlessly across different cloud providers through a unified interface. No knowledge of cloud offerings is required or expected – you simply define the workload and its resource requirements, and SkyPilot will automatically execute it on AWS, Google Cloud Platform or Microsoft Azure. | ||
|
||
<!-- TODO: We need a logo here --> | ||
### Key features | ||
* **Run existing projects on the cloud** with zero code changes | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* **No cloud lock-in** – seamlessly run your code across different cloud providers (AWS, Azure or GCP) | ||
* **Automatic recovery from spot instance failures** | ||
* **Automatic fail-over** to find resources across regions and clouds | ||
* **Store datasets on the cloud** and access them like you would on a local file system | ||
* **Easily manage job queues** across multiple clusters and **automatically stop idle clusters** | ||
|
||
|
||
## Getting Started | ||
Please refer to our [documentation](https://sky-proj-sky.readthedocs-hosted.com/en/latest/). | ||
You can find our documentation [here](https://sky-proj-sky.readthedocs-hosted.com/en/latest/). | ||
- [Installation](https://sky-proj-sky.readthedocs-hosted.com/en/latest/getting-started/installation.html) | ||
- [Quickstart](https://sky-proj-sky.readthedocs-hosted.com/en/latest/getting-started/quickstart.html) | ||
- [CLI](https://sky-proj-sky.readthedocs-hosted.com/en/latest/reference/cli.html) | ||
|
||
## Developer Guide | ||
### Setup | ||
Use editable mode (`-e`) when installing: | ||
```bash | ||
# SkyPilot requires python >= 3.6 and < 3.10. | ||
# You can just install the dependencies for | ||
# certain clouds, e.g., ".[aws,azure,gcp]" | ||
pip install -e ".[all]" | ||
pip install -r requirements-dev.txt | ||
``` | ||
IMPORTANT: Please `export SKYPILOT_DEV=1` before running the CLI commands in the terminal, so that developers' usage logs do not pollute the actual user logs. | ||
|
||
|
||
### Submitting pull requests | ||
- After you commit, format your code with [`format.sh`](./format.sh). | ||
- In the PR description, write a `Tested:` section to describe relevant tests performed. | ||
- For changes that touch the core system, run the [smoke tests](#testing) and ensure they pass. | ||
- Follow the [Google style guide](https://google.github.io/styleguide/pyguide.html). | ||
|
||
|
||
### Environment variables for developers | ||
- `export SKYPILOT_DEV=1` to send usage logs to dev space. | ||
- `export SKYPILOT_DISABLE_USAGE_COLLECTION=1` to disable usage logging. | ||
- `export SKYPILOT_DEBUG=1` to show debugging logs (use logging.DEBUG level). | ||
- `export SKYPILOT_MINIMIZE_LOGGING=1` to minimize the logging for demo purpose. | ||
|
||
### Dump timeline | ||
|
||
Timeline is useful for performance analysis and debugging in SkyPilot. | ||
|
||
Here are the APIs: | ||
|
||
```python | ||
|
||
from utils import timeline | ||
- [CLI reference](https://sky-proj-sky.readthedocs-hosted.com/en/latest/reference/cli.html) | ||
|
||
## Example SkyPilot Task | ||
|
||
# record a function in the timeline with the function path name | ||
@timeline.event | ||
def f(): ... | ||
Tasks in SkyPilot are specified as a YAML file containing the resource requirements, data to be synced, setup commands and the task commands. Here is an example. | ||
|
||
```yaml | ||
# my-task.yaml | ||
resources: | ||
# 1x NVIDIA V100 GPU | ||
accelerators: V100:1 | ||
|
||
# record a function in the timeline using name='my_name' | ||
@timeline.event(name='event_name') | ||
def f(): ... | ||
# Number of VMs to launch in the cluster | ||
num_nodes: 1 | ||
|
||
# Working directory (optional) containing the project codebase. | ||
# Its contents are synced to ~/sky_workdir/ on the cluster. | ||
workdir: . | ||
|
||
# record an event over a code block in the timeline: | ||
with timeline.Event(name='event_name'): | ||
... | ||
# Commands to be run before executing the job | ||
# Typical use: pip install -r requirements.txt, git clone, etc. | ||
setup: | | ||
echo "Running setup." | ||
|
||
# use a file lock with event: | ||
with timeline.FileLockEvent(lockpath): | ||
pass | ||
# Commands to run as a job | ||
# Typical use: make use of resources, such as running training. | ||
run: | | ||
echo "Hello, SkyPilot!" | ||
conda env list | ||
``` | ||
|
||
To dump the timeline, set environment variable `SKYPILOT_TIMELINE_FILE_PATH` to a file path. | ||
|
||
View the dumped timeline file using `Chrome` (chrome://tracing) or [Perfetto](https://ui.perfetto.dev/). | ||
|
||
### Updating the SkyPilot docker image | ||
1. Authenticate with SkyPilot ECR repository. Contact romil.bhardwaj@berkeley.edu for access: | ||
``` | ||
aws ecr-public get-login-password --region us-east-1 | docker login --username AWS --password-stdin public.ecr.aws/a9w6z7w5 | ||
``` | ||
|
||
2. Build and tag the docker image: | ||
``` | ||
docker build -t public.ecr.aws/a9w6z7w5/sky:latest . | ||
``` | ||
|
||
3. Push the image to ECR: | ||
``` | ||
docker push public.ecr.aws/a9w6z7w5/sky:latest | ||
``` | ||
|
||
### Some general engineering practice suggestions | ||
|
||
These are suggestions, not strict rules to follow. When in doubt, follow the [style guide](https://google.github.io/styleguide/pyguide.html). | ||
|
||
* Use `TODO(author_name)`/`FIXME(author_name)` instead of blank `TODO/FIXME`. This is critical for tracking down issues. You can write TODOs with your name and assign it to others (on github) if it is someone else's issue. | ||
* Delete your branch after merging it. This keeps the repo clean and faster to sync. | ||
* Use an exception if this is an error. Only use `assert` for debugging or proof-checking purpose. This is because exception messages usually contain more information. | ||
* Use modern python features and styles that increases code quality. | ||
* Use f-string instead of `.format()` for short expressions to increase readability. | ||
* Use `class MyClass:` instead of `class MyClass(object):`. The later one was a workaround for python2.x. | ||
* Use `abc` module for abstract classes to ensure all abstract methods are implemented. | ||
* Use python typing. But you should not import external objects just for typing. Instead, import typing-only external objects under `if typing.TYPE_CHECKING:`. | ||
|
||
### Testing | ||
To run smoke tests: | ||
``` | ||
bash tests/run_smoke_tests.sh | ||
|
||
# Run one of the smoke tests | ||
bash tests/run_smoke_tests.sh test_minimal | ||
``` | ||
|
||
For profiling code, use: | ||
``` | ||
pip install tuna # Tuna for viz | ||
python3 -m cProfile -o sky.prof -m sky.cli status # Or some other command | ||
tuna sky.prof | ||
This task can be launched on the cloud with the `sky launch` command. | ||
```bash | ||
$ sky launch my-task.yaml | ||
``` | ||
SkyPilot will perform multiple functions for you: | ||
1. Find the lowest priced VM instance type across different clouds | ||
2. Provision the VM | ||
3. Copy the local contents of `workdir` to the VM | ||
4. Run the task's `setup` commands to prepare the VM for running the task | ||
5. Run the task's `run` commands | ||
|
||
<!---- TODO(romilb): Example GIF goes here ----> | ||
Please refer to the [quick start](https://sky-proj-sky.readthedocs-hosted.com/en/latest/getting-started/quickstart.html) and [documentation](https://sky-proj-sky.readthedocs-hosted.com/en/latest/) for more on how to use SkyPilot. | ||
|
||
## Contributing | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Please refer to the [contribution guide](CONTRIBUTING.md). | ||
romilbhardwaj marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add:
Please read through this document before contributing so that there is all the necessary information needed for us to effectively respond to your contribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - makes the flow better. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence unfortunately reads a bit too direct to me. I'd love to not include it. It is implied anyway.