Skip to content

Commit 90f87b8

Browse files
sarayourfriendkrysalzackkrida
authored
Expand ov documentation and replace environment setup docs with ov instructions (#4567)
* Fix broken returns in stdout/err * Greatly expand built-in `ov` documentation * Revert "Fix broken returns in stdout/err" This reverts commit 3210124. * Do not provision docker tty when in a pipeline * Update documentation for ov Do not use a relative path. While it might be generally more consistent, it is much more cumbersome, and would require stating "from the repo root" each time to be accurate * Fix broken links * Replace references to old quickstart guide * Explain blobless clone * Update documentation/api/guides/quickstart.md Co-authored-by: zack <[email protected]> * Reword for clarity * Fix incorrect or muddied usages of "dependent" * Update link to non-existent prod subdomain * Add `ov doctor` --------- Co-authored-by: Krystle Salazar <[email protected]> Co-authored-by: zack <[email protected]>
1 parent b3e5673 commit 90f87b8

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+526
-797
lines changed

Diff for: .devcontainer/devcontainer.json

-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
// More info: https://containers.dev/features
1010
// List of features and their default options: https://github.com/devcontainers/features/tree/main/src
1111
"features": {
12-
// Openverse development environment requirement matrix: https://docs.openverse.org/general/general_setup.html#requirement-matrix
1312
"ghcr.io/guiyomh/features/just:0": {},
1413
"ghcr.io/devcontainers/features/python:1": {
1514
// Match the Python version defined in `*/Pipfile`.

Diff for: .github/ISSUE_TEMPLATE/new_source_suggestion.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ body:
7171
- The script must not use anything from `catalog/dags/retired/providers/provider_api_scripts/modules/etlMods.py`, since that module is deprecated.
7272
- If the provider API has can be queried by 'upload date' or something similar, the script should take a `--date` parameter when run as a script, giving the date for which we should collect images. The form should be `YYYY-MM-DD` (so, the script can be run via `python my_favorite_provider.py --date 2018-01-01`).
7373
- The script must provide a main function that takes the same parameters as from the CLI. In our example from above, we'd then have a main function `my_favorite_provider.main(date)`. The main should do the same thing calling from the CLI would do.
74-
- The script *must* conform to [PEP8][pep8]. Please use `./ov just lint` before committing.
74+
- The script *must* conform to [PEP8][pep8]. Please use `ov just lint` before committing.
7575
- The script should use small, testable functions.
7676
- The test suite for the script may break PEP8 rules regarding long lines where appropriate (e.g., long strings for testing).
7777

Diff for: .github/pull_request_template.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ Fixes #[issue number] by @[issue author]
2424
- [ ] I added or updated tests for the changes I made (if applicable).
2525
- [ ] I added or updated documentation (if applicable).
2626
- [ ] I tried running the project locally and verified that there are no visible errors.
27-
- [ ] I ran the DAG documentation generator (`./ov just catalog/generate-docs` for catalog
28-
PRs) or the media properties generator (`./ov just catalog/generate-docs media-props`
29-
for the catalog or `./ov just api/generate-docs` for the API) where applicable.
27+
- [ ] I ran the DAG documentation generator (`ov just catalog/generate-docs` for catalog
28+
PRs) or the media properties generator (`ov just catalog/generate-docs media-props`
29+
for the catalog or `ov just api/generate-docs` for the API) where applicable.
3030

3131
[best_practices]:
3232
https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines

Diff for: .gitignore

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ logs/
1313

1414
# Environment
1515
.env
16-
.ov_aliases.json
16+
.ov_profile.json
1717

1818
# Python environments
1919
env/

Diff for: .vale/README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ the Openverse monorepo, refer to the README at
1212
To run Vale with Openverse's configuration, use the just recipe:
1313

1414
```bash
15-
./ov just .vale/run
15+
ov just .vale/run
1616
```
1717

1818
This recipe _always_ builds Vale. The Openverse Vale docker image is fast to

Diff for: api/justfile

+2-2
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,11 @@ recreate:
8787
########
8888

8989
# Make a cURL GET request to service at the given path
90-
_curl-get path origin="http://localhost:50280":
90+
@_curl-get path origin="http://localhost:50280":
9191
curl "{{ origin }}/v1/{{ path }}"
9292

9393
# Make a test cURL GET request to the API
94-
stats media="images" origin="http://localhost:50280":
94+
@stats media="images" origin="http://localhost:50280":
9595
just _curl-get "{{ media }}/stats/" {{ origin }}
9696

9797
# Launch a `pgcli` shell in the web container

Diff for: catalog/templates/template_test.py_template

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ TODO: Add additional tests for any methods you added in your subclass.
33
Try to test edge cases (missing keys, different data types returned, Nones, etc).
44
You may also need to update the given test names to be more specific.
55

6-
Run your tests locally with `./ov just catalog/test -k {provider_underscore}`
6+
Run your tests locally with `ov just catalog/test -k {provider_underscore}`
77
"""
88

99
import json

Diff for: catalog/tests/dags/providers/provider_api_scripts/test_cc_mixter.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Run these tests locally with `./ov just catalog/test -k cc_mixter`"""
1+
"""Run these tests locally with `ov just catalog/test -k cc_mixter`"""
22

33
import json
44
from pathlib import Path

Diff for: catalog/tests/dags/providers/provider_api_scripts/test_inaturalist.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
# to pull a sample of records without breaking referential integrity.
3838
# - Putting your final zipped test files in /tests/s3-data/inaturalist-open-data
3939
# so that they will be synced over to minio.
40-
# - Run `./ov just down -v` and then `./ov just recreate` to make sure that the test data gets to
40+
# - Run `ov just down -v` and then `ov just recreate` to make sure that the test data gets to
4141
# the test s3 instance. That process may take on the order of 15 minutes for the full
4242
# dataset. You'll know that it's done when the s3-load container in docker exits.
4343
# - Then, in airflow, trigger the dag possibly with configuration

Diff for: docker/dev_env/exec.py

+89-9
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,111 @@
22
import os
33
import sys
44
from pathlib import Path
5+
from textwrap import dedent
56

67

78
SHARED_ALIASES = {
8-
"j": ["just"],
9-
"nuxt": ["j", "p", "frontend", "dev"],
9+
"j": {
10+
"cmd": ["just"],
11+
"doc": "A common alias for `just` to save three keystrokes (it adds up!)",
12+
},
13+
"nuxt": {
14+
"cmd": ["j", "p", "frontend", "dev"],
15+
"doc": "Run the Openverse frontend local development site",
16+
},
1017
}
1118

1219

20+
# Document alias usage and configuration here. Including it
21+
# in the command makes it easy to discover and noticeable when
22+
# making changes, so it will not go out of sync.
23+
# ov aliases is included in the root `ov` script to help make this visible
24+
# and is mentioned in the general setup documentation
25+
ALIAS_USAGE = """
26+
ov aliases
27+
28+
USAGE
29+
ov ALIAS [...args]
30+
Run an aliased command configured for ov.
31+
32+
ov aliases [--help|-h]
33+
List all available aliases. If --help is passed, display this help message instead.
34+
35+
DESCRIPTION
36+
`ov init` creates a ./ov_profile.json file at the root of the repository.
37+
The "aliases" key of this JSON file should be a dictionary, with alias
38+
configuration objects as the keys. The configuration object is a dictionary with
39+
a "cmd" key assigned to a list of command arguments to expand the alias to.
40+
You may also provide a "doc" key to document the alias. The documentation for
41+
each alias is displayed by running `ov aliases`.
42+
43+
To run an alias, pass it as the command argument to ov. To run the built-in "j"
44+
alias, run `ov j`.
45+
46+
Only the first argument passed to ov will be expanded using aliases. Aliases can
47+
stack (that is, repeatedly expand the first argument, if an alias refers to another alias)
48+
but only ever the leading argument of the expanded alias. For example, the built-in
49+
alias "nuxt" expands to "j p frontend dev", and ov will expand "j" to "just". However,
50+
if an alias "p", "frontend", or "dev" existed, ov will not expand those, as they are
51+
not the leading argument of the expanded arguments list.
52+
""".strip()
53+
54+
55+
def alias_info(aliases: dict, args: list[str]):
56+
if "--help" in args or "-h" in args:
57+
print(ALIAS_USAGE)
58+
return
59+
60+
# else, assume --list
61+
output = "ov aliases\n"
62+
longest_alias = len(sorted(aliases.keys())[-1])
63+
for alias, cmd in aliases.items():
64+
if isinstance(cmd, dict):
65+
doc = cmd["doc"]
66+
cmd = cmd["cmd"]
67+
else:
68+
doc = "No documentation found."
69+
70+
joined = " ".join(cmd)
71+
output += dedent(
72+
f"""
73+
{alias}{' ' * (longest_alias - len(alias))} -> {joined}
74+
{doc}
75+
{'(Built-in alias)' if alias in SHARED_ALIASES else '(Personal alias)'}
76+
"""
77+
)
78+
79+
print(output)
80+
81+
1382
def expand_aliases(args: list[str]):
14-
ov_aliases = Path(os.getenv("OPENVERSE_PROJECT")) / ".ov_aliases.json"
15-
aliases = SHARED_ALIASES
16-
if ov_aliases.is_file():
17-
aliases |= json.loads(ov_aliases.read_text())
83+
ov_profile = Path(os.getenv("OPENVERSE_PROJECT")) / ".ov_profile.json"
84+
aliases = SHARED_ALIASES.copy()
85+
if ov_profile.is_file():
86+
aliases |= json.loads(ov_profile.read_text()).get("aliases", {})
1887

1988
args = sys.argv[1:]
2089

90+
if args[0] == "aliases":
91+
return alias_info(aliases, args)
92+
2193
while args[0] in aliases:
22-
args = aliases.pop(args[0]) + args[1:]
94+
alias = aliases.pop(args[0])
95+
if isinstance(alias, dict):
96+
args = alias["cmd"] + args[1:]
97+
else:
98+
args = alias + args[1:]
2399

24100
return args
25101

26102

27103
if __name__ == "__main__":
28104
args = expand_aliases(sys.argv[1:])
29105
try:
30-
os.execvp(args[0], args)
106+
if args is not None:
107+
os.execvp(args[0], args)
31108
except FileNotFoundError:
32-
sys.stdout.buffer.write(args[0].encode() + b": command not found\n")
109+
print(f"{args[0]}: command not found")
110+
exit(1)
111+
else:
112+
exit(0)

Diff for: docker/dev_env/run.sh

+6-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ shared_args=(
2626
run_args=(
2727
-d
2828
--name "$container_name"
29+
# Use host network so things like Sphinx and the frontend that run directly in `ov`
30+
# will have ports available on the host (and this way we don't have to manually map each one)
2931
--network host
3032
# Bind the repo to the same exact location inside the container so that pre-commit
3133
# and others don't get confused about where files are supposed to be
@@ -44,7 +46,10 @@ run_args=(
4446
# environment doesn't have one to attach.
4547
# In other words, only tell Docker to attach a TTY to the container when
4648
# there's one to attach in the first place.
47-
if [ -t 0 ]; then
49+
# Additionally, we only want to attach a TTY if the command is actually interactive
50+
# and not, instead, as part of a pipe.
51+
# -t 1 is false if 1 is not stdout (it's something else! a file, a pipe, etc)
52+
if [[ -t 0 && -t 1 ]]; then
4853
shared_args+=(-t)
4954
fi
5055

Diff for: docker/minio/load_to_s3_entrypoint.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
22
# This does *not* allow for testing permissions issues that may come up in real AWS.
3-
# And, if you remove files from /tests/s3-data, you will need to use `./ov just down -v`
4-
# and `./ov just up` or `./ov just recreate` to see the minio bucket without those files.
3+
# And, if you remove files from /tests/s3-data, you will need to use `ov just down -v`
4+
# and `ov just up` or `ov just recreate` to see the minio bucket without those files.
55
# Loop through subdirectories mounted to the volume and load them to s3/minio.
66
# This takes care of filesystem delays on some local dev environments that may make
77
# minio miss files included directly in the minio volume.

Diff for: documentation/api/guides/https.md

+17-10
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,34 @@ Make sure you have gone through the
1010
[quickstart guide](/api/guides/quickstart.md) before attempting this as this is
1111
a slightly more advanced process.
1212

13-
Additionally, you will need to install
14-
[mkcert](/general/general_setup.md#mkcert) as described in the
15-
[general setup guide](/general/general_setup.md).
13+
Additionally, you will need to install `mkcert`.
14+
[Follow `mkcert`'s installation guide](https://github.com/FiloSottile/mkcert?tab=readme-ov-file#installation)
15+
to do so.
16+
17+
```{caution}
18+
`ov` does not yet support mkcert. You must run this command without `ov`
19+
on your host system for it to work.
20+
```
1621

1722
## Steps
1823

1924
1. Create certificates for NGINX to use.
2025

26+
```{caution}
27+
Run this on your host system, `ov` does not support mkcert.
28+
```
29+
2130
```bash
22-
./ov just docker/nginx/cert
31+
just docker/nginx/cert
2332
```
2433

2534
This will create a certificate file `openversse.crt` and a key file
2635
`openverse.key` in the `docker/nginx/certs/` directory.
2736

28-
2. Bring the ingestion server and API up, along with all their dependent
29-
services.
37+
2. Start the API along with its dependencies:
3038

3139
```bash
32-
./ov just api/up
40+
ov just api/up
3341
```
3442

3543
The `api/up` recipe orchestrates the following services: `cache`, `db`,
@@ -41,8 +49,7 @@ Additionally, you will need to install
4149
3. Make an API call over HTTPS.
4250

4351
```bash
44-
./ov just api/stats images https://localhost:50243
45-
./ov just _curl-get "images/stats/" https://localhost:50243
46-
curl "https://localhost:50243/v1/images/stats/"
52+
ov just api/stats images https://localhost:50243
53+
ov just _curl-get "images/stats/" https://localhost:50243
4754
[{"source_name":"flickr","display_name":"Flickr","source_url":"https://www.flickr.com","logo_url":null,"media_count":2500},{"source_name":"stocksnap","display_name":"StockSnap","source_url":"https://stocksnap.io","logo_url":null,"media_count":2500}]%
4855
```

0 commit comments

Comments
 (0)