Skip to content

Commit

Permalink
Merge pull request #951 from TanyaStere42/Update-contribution-instruc…
Browse files Browse the repository at this point in the history
…tions

removed references to develop, clarified pr workflow
  • Loading branch information
TanyaStere42 committed Feb 7, 2024
2 parents 2a656c6 + 3fed5fd commit 415bf17
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 235 deletions.
15 changes: 0 additions & 15 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
<!--
Pre-Pull Request Checklist
- Ensure that the PR is properly rebased
- Example: The PR is rebased on `develop` (commit: `<insert develop commit hash>`)
- Use `/rebase` as a PR comment to rebase it once created
- Ensure that the PR uses a consolidated database migration
- Example: One database migration is proposed (revision `<insert develop revision>` -> `<insert new revision>`)
- Ensure that the PR is properly sanitized
- Example: No sensitive data or large content was added to this PR
-->


## Pull Request Overview

- Specify a list of summary items that this PR is accomplishing
Expand Down
163 changes: 109 additions & 54 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,16 @@

All contributions are welcome and encouraged. There are a few guidelines and styling aspects that we require and encourage you to use so that we might see this project through many years of successful development.

## Development Guidelines
## Pull Request Considerations

### Pull Request Checklist
Details on how to configure and pass each of these required checks is detailed in the sections in this guideline section.

To submit a pull request (PR) to Houston, we require the following standards to be enforced. Details on how to configure and pass each of these required checks is detailed in the sections in this guideline section.
### Pre-commit
We require code formatting with Brunette (a *better* version of Black) and linted with Flake8 using the pre-commit (includes automatic checks with brunette and flake8 plus includes other general line-ending, single-quote strings, permissions, and security checks). Pre-commit is installed once and then runs automatically.

* **Ensure that the PR is properly formatted**
* We require code formatting with Brunette (a *better* version of Black) and linted with Flake8 using the pre-commit (includes automatic checks with brunette and flake8 plus includes other general line-ending, single-quote strings, permissions, and security checks)
```
# Command Line Example
pre-commit run --all-files
```
* **Ensure that the PR is properly rebased**
* We require new feature code to be rebased onto the latest version of the `develop` branch.
```
git fetch -p
git checkout <feature-branch>
git diff origin/<feature-branch>.. # Check there's no difference between local branch and remote branch
git rebase -i origin/main # "Pick" all commits in feature branch
# Resolve all conflicts
git log --graph --oneline --decorate origin/main HEAD # feature-branch commits should be on top of origin/develop
git show --reverse origin/main.. # Check the changes in each commit if necessary
git push --force origin <feature-branch>
```
* **Ensure that the PR uses a consolidated database migration**
* We require new any database migrations (optional) with Alembic are consolidated and condensed into a single file and version. Exceptions to this rule (allowing possibly up to 3) will be allowed after extensive discussion and justification.
* All database migrations should be created using a downgrade revision that matches the existing revision used on the `develop` branch. Further,a PR should never be merged into develop that contains multiple revision heads.
### Consolidated database migration
Database migrations (optional) with Alembic are consolidated and condensed into a single file and version. Exceptions to this rule (allowing possibly up to 3) will be allowed after extensive discussion and justification.
All database migrations should be created using a downgrade revision that matches the existing revision used on the `develop` branch. Further,a PR should never be merged into develop that contains multiple revision heads.
```
invoke app.db.downgrade <develop branch revision ID>
rm -rf migrations/versions/<new migrations>.py
Expand All @@ -37,21 +20,24 @@ To submit a pull request (PR) to Houston, we require the following standards to
invoke app.db.history # Check that the history matches
invoke app.db.heads # Ensure that there is only one head
```
* **Ensure that the PR is properly tested**
* We require new feature code to be tested via Python tests and simulated REST API tests. We use PyTest to ensure that your code is working cohesively and that any new functionality is exercised.
* We require new feature code to also be fully compatible with a containerized runtime environment like Docker.

### Testing
Feature code is tested via Python tests and simulated REST API tests. We use PyTest to ensure that your code is working cohesively and that any new functionality is exercised.
Feature code to also be fully compatible with a containerized runtime environment like Docker.
```
pytest
```
* **Ensure that the PR is properly covered**
* We require new feature code to be tested (previous item) and that the percentage of the code covered by tests does not decrease. We use PyTest Coverage and CodeCov.io to ensure that your code is being properly covered by new tests.
* To export a HTML report of the current coverage statistics, run the following:

### Test coverage
New feature code should have automated tests, and the percentage of the code covered by tests does not decrease with the addition of new features. We use PyTest Coverage and CodeCov.io to ensure that your code is being properly covered by new tests.
To export a HTML report of the current coverage statistics, run the following:
```
pytest -s -v --cov=./ --cov-report=html
open _coverage/html/index.html
```
* **Ensure that the PR is properly sanitized**
* We require the PR to not include large files (images, database files, etc) without using [GitHub Large File Store (LFS)](https://git-lfs.github.com).

### Large file sanitation
PRs should not include large files (images, database files, etc) without using [GitHub Large File Store (LFS)](https://git-lfs.github.com).
```
git lfs install
git lfs track "*.png"
Expand All @@ -60,11 +46,7 @@ To submit a pull request (PR) to Houston, we require the following standards to
git commit -m "Add new image file"
git push
```
* We also require any sensitive code, configurations, or pre-specified values be omitted, truncated, or redacted. For example, the file `_db/secrets/py` is not committed into the repository and is ignored by `.gitignore`.
* **Ensure that the PR is properly reviewed**
* After the preceding checks are satisfied, the code is ready for review. All PRs are required to be reviewed and approved by at least one registered contributor or administrator on the Houston project.
* When the PR is created in GitHub, make sure that the repository is specified as `WildMeOrg/houston` and not its original fork. Further, make sure that the base branch is specified as `develop` and not `master`.

Any sensitive code, configurations, or pre-specified values should be omitted, truncated, or redacted. For example, the file `_db/secrets/py` is not committed into the repository and is ignored by `.gitignore`.

## Code Style

Expand All @@ -91,10 +73,55 @@ Reference [pre-commit's installation instructions](https://pre-commit.com/#insta
See `.pre-commit-config.yaml` for a list of configured linters and fixers.

## Development Environment
### Install from source for troubleshooting

Installation of Houston and the other components of Codex from source is intended to facilitate development leveraging the docker-compose environment. Full deployment of Codex outside docker-compose orchestration is not supported, and any changes should not be considered finished until they have been tested in the docker-compose environment.

#### Clone the Project

```bash
git clone --recurse-submodules https://github.com/USERNAME/houston.git
cd houston/
```
#### Setup Codex Environment

It is recommended to use virtualenv or Anaconda/Miniconda to manage Python
dependencies. Please, learn details yourself.
For quickstart purposes the following will set up a virtualenv for you:

```bash
./scripts/codex/venv.sh
source virtualenv/houston3.7/bin/activate

# To add bash-completion
export SCRIPT="$(pwd)/.invoke-completion.sh"
invoke --print-completion-script bash > $SCRIPT
echo "source $SCRIPT" >> virtualenv/houston3.7/bin/activate
```

Set up and install the package:

```bash
invoke dependencies.install
```

#### Run Server

NOTE: All dependencies and database migrations will be automatically handled,
so go ahead and turn the server ON! (Read more details on this in Tips section)

```bash
export HOUSTON_APP_CONTEXT=codex
$ invoke app.run
```

#### Deploy Server

In general, you deploy this app as any other Flask/WSGI application. There are
a few basic deployment strategies documented in the [`./deploy/`](./deploy/)
folder.

It's recommended that you install the Houston application and its dependencies [using the Docker container environment](#container-installation). If you'd like to make things difficult you can also look at the [local installation instructions](#local-installation).

<a href="local-installation"></a>
### Installing and running on your local system

#### PyInvoke installation
Expand Down Expand Up @@ -143,27 +170,55 @@ invoke dependencies.install
invoke codex.run
```

#### Usage
### Usage Tips

<a href="container-installation"></a>
### Installing and running with Docker
Once you have invoke, you can learn all available commands related to this
project from:

These instructions will install and run the application and dependent application.
```bash
$ invoke --list
```

The configuration is by environment variable via the `.env` file in this directory.
Learn more about each command with the following syntax:

For development purposes, this setup exposes each of the services as follows:
```bash
$ invoke --help <task>
```

<!-- don't use port 80 when defining any hosts -->
- EDM - http://localhost:81/
- Sage (Wildbook-IA) - http://localhost:82/
- Houston - http://localhost:83/houston/
- CODEX (frontend) - http://localhost:84/
- CODEX (api docs) - http://localhost:84/api/v1/
- GitLab - http://localhost:85
For example:

See also the results of `docker-compose ps -a`, which will list the services and the ports exposed on those services.
```bash
$ invoke --help codex.run
Usage: inv[oke] [--core-opts] codex.run [--options] [other tasks here ...]

Docstring:
Run DDOTS RESTful API Server.

Options:
-d, --[no-]development
-h STRING, --host=STRING
-i, --[no-]install-dependencies
-p, --port
-u, --[no-]upgrade-db
```

#### Usage
Use the following command to enter ipython shell (`ipython` must be installed):

```bash
$ invoke app.env.enter
```

`codex.run` and `app.env.enter` tasks automatically prepare all dependencies
(using `pip install`) and migrate database schema to the latest version.

Database schema migration is handled via `app.db.*` tasks group. The most
common migration commands are `app.db.upgrade` (it is automatically run on
`codex.run`), and `app.db.migrate` (creates a new migration).

You can use [`better_exceptions`](https://github.com/Qix-/better-exceptions)
package to enable detailed tracebacks. Just add `better_exceptions` to the
`app/requirements.txt` and `import better_exceptions` in the `app/__init__.py`.

##### Running the applications

Expand Down Expand Up @@ -290,7 +345,7 @@ in the `docker-compose.yml` by altering the `dev-frontend` volume mapping `- _fr
More details about Codex front end contribution are outside the scope of this README but can be found here: [**codex-frontend**](https://github.com/WildMeOrg/codex-frontend)

##### EDM
The EDM is a compiled Java application, and no volume mapping solution to a running Docker container is available at this time.
The EDM is presently maintained for migration of production platforms from Wildbook to Codex. It will be removed when production platforms have all been migrated.

### Testing

Expand Down
Loading

0 comments on commit 415bf17

Please sign in to comment.