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

Fix just argument splitting on pass-through recipes with complex args #4961

Merged
merged 7 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vale/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ IGNORE_FILES_PATTERN := ".pnpm|changelogs|projects/proposals|node_modules|.?venv
# of the current working directory. If a custom separator is specified, newlines are
# replaced with this separator before outputting the file list.
_files separator="\n":
#! /usr/bin/env sh
#! /usr/bin/env bash
files=$(
find "$PWD/.." -type f \( -name "*.md" -o -name "*.mdx" \) \
| grep -vE "{{ IGNORE_FILES_PATTERN }}"
Expand Down
22 changes: 14 additions & 8 deletions api/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ install *args:
######

# Bring up services specific to the API profile
[positional-arguments]
up *flags:
env COMPOSE_PROFILES="api" just ../up {{ flags }}
env COMPOSE_PROFILES="api" just ../up "$@"

# Bring up services specific to the API profile, in addition to the API server
[positional-arguments]
up-extra *flags:
env COMPOSE_PROFILES="api,api_extra" just ../up {{ flags }}
env COMPOSE_PROFILES="api,api_extra" just ../up "$@"

# Wait for all profile services to be up
wait-up: up
Expand Down Expand Up @@ -103,12 +105,14 @@ pgcli db_user_pass="deploy" db_name="openledger": up
#########################

# Run Django administrative commands locally
dj-local +args="":
pdm run python manage.py {{ args }}
[positional-arguments]
dj-local *args:
pdm run python manage.py "$@"

# Run Django administrative commands inside the Docker container
dj +args="": wait-up
env DC_USER="{{ env_var_or_default("DC_USER", "opener") }}" just ../exec web python manage.py {{ args }}
[positional-arguments]
dj *args: wait-up
env DC_USER="{{ env_var_or_default("DC_USER", "opener") }}" just ../exec web python manage.py "$@"

# Get IPython shell inside the Docker container
ipython:
Expand Down Expand Up @@ -152,12 +156,14 @@ generate-docs doc="media-props" fail_on_diff="true":
#########

# Run API tests inside the Docker container
[positional-arguments]
test *args: wait-up
env DC_USER="opener" just ../exec web pytest {{ args }}
env DC_USER="opener" just ../exec web pytest "$@"

# Run API tests locally
[positional-arguments]
test-local *args:
pdm run pytest {{ args }}
pdm run pytest "$@"

# Run smoke test for the API docs
doc-test: wait-up
Expand Down
31 changes: 13 additions & 18 deletions automations/js/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ NO_COLOR := "\\033[0m"
just --list --unsorted

# Run a script
[positional-arguments]
run script *args:
pnpm --filter automations exec node src/{{ script }} {{ args }}
#! /usr/bin/env bash
pnpm --filter automations exec node src/{{ script }} "${@:2}"

#################
# Weekly report #
Expand All @@ -25,21 +27,14 @@ report:
# Render #
##########

_render slug name:
just run render-jinja.js \
templates/release-drafter.yml.jinja \
.github/release-drafter-{{ slug }}.yml \
'{"app": "{{ name }}"}'

render-release-drafter:
#! /usr/bin/env sh
render_release_drafter() {
slug=$1
name=$2
just run render-jinja.js \
templates/release-drafter.yml.jinja \
.github/release-drafter-$slug.yml \
"'{ \"app\": \"$name\" }'"
}
render_release_drafter api API
render_release_drafter ingestion_server "Ingestion Server"
render_release_drafter frontend Frontend
render_release_drafter catalog Catalog

# Render all templates (shortcut for easy iteration)
render-templates:
just render-release-drafter
just _render api API
just _render ingestion_server "Ingestion Server"
just _render frontend Frontend
just _render catalog Catalog
6 changes: 4 additions & 2 deletions automations/python/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ NO_COLOR := "\\033[0m"


# Install dependencies
[positional-arguments]
install *args:
pdm install {{ args }}
pdm install "$@"

# Run a script
[positional-arguments]
run script *args:
pdm run {{ script }} {{ args }}
pdm run "$@"
29 changes: 18 additions & 11 deletions catalog/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,14 @@ install *args: check-py-version
######

# Bring up services specific to the catalog profile
[positional-arguments]
up *flags:
env COMPOSE_PROFILES="catalog" just ../up {{ flags }}
env COMPOSE_PROFILES="catalog" just ../up "$@"

# Bring up services specific to the catalog profile, except Airflow
[positional-arguments]
up-deps *flags:
env COMPOSE_PROFILES="catalog_dependencies" just ../up {{ flags }}
env COMPOSE_PROFILES="catalog_dependencies" just ../up "$@"

# Load sample data into upstream DB
init: up
Expand All @@ -82,11 +84,12 @@ shell:
env DC_USER="airflow" just ../exec {{ SERVICE }} /bin/bash

# Launch an IPython shell in a new container under `SERVICE`
[positional-arguments]
ipython *args: up-deps
env DC_USER="airflow" just ../run \
--workdir /opt/airflow/catalog/dags \
{{ SERVICE }} \
bash -c \'ipython {{ args }}\'
bash -c "ipython ${@:2}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sarayourfriend why was ${@:2} used here? There doesn't appear to be a positional argument that we need to ignore, and it seems like the first argument after the command provided would be swallowed (for instance ov just catalog/ipython --TerminalInteractiveShell.editing_mode=vi would no longer work as expected because the editing_mode argument would be ignored). Was that intentional?


# Launch a `pgcli` shell in the PostgreSQL container
pgcli db_user_pass="deploy" db_name="openledger": up
Expand All @@ -97,30 +100,32 @@ pgcli db_user_pass="deploy" db_name="openledger": up
#########

# Run a command in a test container under `SERVICE`
_mount-test command: up-deps
[positional-arguments]
_mount-test *command: up-deps
env DC_USER="airflow" just ../run \
-e AIRFLOW_VAR_INGESTION_LIMIT=1000000 \
-w /opt/airflow/catalog \
--volume \'{{ justfile_directory() }}/../docker\':/opt/airflow/docker/ \
--volume {{ justfile_directory() }}/../docker:/opt/airflow/docker/ \
{{ SERVICE }} \
{{ command }}
"$@"

# Launch a Bash shell in a test container under `SERVICE`
# Run pytest with `--pdb` to workaround xdist breaking pdb.set_trace()
test-session:
just _mount-test bash

# Run pytest in a test container under `SERVICE`
[positional-arguments]
test *args:
just _mount-test "bash -c \'pytest {{ args }}\'"
just _mount-test bash -c "pytest $@"

#############
# Utilities #
#############

# Generate the documentation (either "dag" or "media-props")
generate-docs doc="dag" fail_on_diff="false":
#!/bin/bash
#! /usr/bin/env bash
set -e
if [ "{{ doc }}" == "dag" ]; then
SCRIPT_PATH="catalog/utilities/dag_doc_gen/dag_doc_generation.py"
Expand All @@ -136,18 +141,20 @@ generate-docs doc="dag" fail_on_diff="false":
echo "Invalid documentation type specified, use \`dag\` or \`media-props\`. Exiting."
exit 1
fi

GENERATED_ABS_PATH="/opt/airflow/catalog/${GENERATED_REL_PATH}"
just ../run \
--volume {{ justfile_directory() }}/../docker:/opt/airflow/docker/ \
-e PYTHONPATH=/opt/airflow/catalog:/opt/airflow/catalog/dags \
{{ SERVICE }} \
"bash -c 'python $SCRIPT_PATH && chmod 666 $GENERATED_ABS_PATH'"
bash -c "python $SCRIPT_PATH && chmod 666 $GENERATED_ABS_PATH"

TEMP="documentation/meta/temp"
mv ../catalog/$GENERATED_REL_PATH ../$TEMP.md
mv ../catalog/"$GENERATED_REL_PATH" ../"$TEMP".md
echo "Moved the generated file to ../$TEMP.md"
echo -n "Running linting..."
# Linting step afterwards is necessary since the generated output differs greatly from what prettier expects
just ../lint prettier $TEMP.md &>/dev/null || true
just ../lint prettier "$TEMP".md &>/dev/null || true
echo "Linting done!"

echo -n "Replacing linted md <hr> '---' with '----' required by sphinx..."
Expand Down
3 changes: 2 additions & 1 deletion docker/cache/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ NO_COLOR := "\\033[0m"
#######

# Open the Redis CLI as an interactive REPL
[positional-arguments]
cli *args:
just ../../exec cache redis-cli {{ args }}
just ../../exec cache redis-cli "$@"

# Delete all data cached in Redis
flushall:
Expand Down
3 changes: 2 additions & 1 deletion documentation/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ live port="50230": clean
pdm run sphinx-autobuild -b html . _serve/ --port {{ port }}

# Compile the documentation into a static site
[positional-arguments]
build *args: clean
pdm run sphinx-build {{ args }} -b html . _build/
pdm run sphinx-build "$@" -b html . _build/

# Serve the compiled docs using a simple HTTP server
serve port="50231":
Expand Down
2 changes: 1 addition & 1 deletion frontend/bin/playwright.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#! /usr/bin/env sh
#! /usr/bin/env bash
set -e

version() {
Expand Down
6 changes: 4 additions & 2 deletions frontend/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ run-img tag="openverse-frontend:local":
######

# Bring up services specific to the frontend profile
[positional-arguments]
up *flags:
env COMPOSE_PROFILES="frontend" just ../up {{ flags }}
env COMPOSE_PROFILES="frontend" just ../up "$@"

# Wait for all profile services to be up
wait-up: up
Expand All @@ -67,8 +68,9 @@ init: wait-up
cd .. && ./setup_plausible.sh

# Run a package.json script via pnpm
[positional-arguments]
run *args:
pnpm run {{ args }}
pnpm run "$@"

# Generate the specified kind of documentation
generate-docs doc="media-props" fail_on_diff="true":
Expand Down
10 changes: 7 additions & 3 deletions indexer_worker/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ install *args="--dev":
######

# Bring up services specific to the indexer worker profile
[positional-arguments]
up *flags:
env COMPOSE_PROFILES="catalog_indexer_worker" just ../up {{ flags }}
env COMPOSE_PROFILES="catalog_indexer_worker" just ../up "$@"

wait-up: up
just wait
Expand Down Expand Up @@ -77,9 +78,12 @@ curl-post data host="localhost:50281":
# Tests #
#########

# Run indexer worker tests in the container
[positional-arguments]
test *args: wait-up
just ../exec catalog_indexer_worker pytest {{ args }}
just ../exec catalog_indexer_worker pytest "$@"

# Run indexer-worker tests locally
[positional-arguments]
test-local *args:
pdm run pytest {{ args }}
pdm run pytest "$@"
Loading
Loading