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

Readme cleanup, docs URL and CONTRIBUTING.md #1010

Merged
merged 14 commits into from
Jul 29, 2022
122 changes: 122 additions & 0 deletions CONTRIBUTING.md
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

Copy link
Collaborator

@michaelzhiluo michaelzhiluo Jul 27, 2022

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.

Copy link
Collaborator Author

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!

Copy link
Member

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.

## Contributing Code
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bit of confusion between the bullet points mentioned in Contributing to SkyPilot and the bullet points below. I thought the doc would be structured as:

  • Contributing Bug Reports
  • Contributing Pull Requests
  • Contributing Test cases
  • etc

Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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):

  • Submitting pull requests
  • Testing
  • Some general engineering practice suggestions
  • Environment variables for developers

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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:

  • Installing
  • Testing
  • Submitting pull requests
  • Some general engineering practice suggestions
  • 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.
romilbhardwaj marked this conversation as resolved.
Show resolved Hide resolved

### Testing
To run smoke tests:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, those should be covered in pip install -r requirements-dev.txt above.. are there any other dependencies?

```
bash tests/run_smoke_tests.sh

# Run one of the smoke tests
bash tests/run_smoke_tests.sh test_minimal
```

For profiling code, use:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
```
148 changes: 49 additions & 99 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,117 +1,67 @@
# SkyPilot

![logo](docs/source/images/SkyPilot-logo-wide.png)
Copy link
Member

Choose a reason for hiding this comment

The 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/

Copy link
Collaborator Author

@romilbhardwaj romilbhardwaj Jul 29, 2022

Choose a reason for hiding this comment

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

Done! Set to 80% width


![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
Binary file added docs/source/images/SkyPilot-logo-wide.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.