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.

Sign up for GitHub

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

chore: update documentation for local setup for running cli commands #301

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: update documentation for local setup for running cli commands #301

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
731232e
Update local-setup.md
jerotire Sep 2, 2024
5e5b1b3
Update local-setup.md
jerotire Sep 2, 2024
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
20 changes: 20 additions & 0 deletions docs/app/local-setup.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,23 @@ SELECT u.login_id, r.description, IF(u.team_id IS NULL, "Selfserve", "Internal")
```

:::

## Executing CLI Commands for API (Batch/Queue/etc)


:::info

The additional environment variables are only required if you are running commands which will interact with AWS.

Example shows the `list` command which will output all available commands; replace `list` with chosen command to run it.

:::

```sh
docker exec -it \
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using docker compose? Then it'll let you reference the friendly names of the containers.

docker compose exec -it api /bin/sh -c "..." for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does mean your current directory needs to be vol-app right? Exiting solution would allow execution from anywhere; unless we prefix with a cd or something.

Copy link
Contributor

@JoshuaLicense JoshuaLicense Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Fair point, but I think documenting less flexible commands is far simpler to maintain for us. We could note that these commands need to be ran from the vol-app root directory?

While it's incredibly minor, we don't control the vol-app-api-1 naming. For example, the separator changed recently from _ to - in Docker Compose v2.

-e AWS_ACCESS_KEY_ID=$AWS_ACCESS_KEY_ID \
-e AWS_SECRET_ACCESS_KEY=$AWS_SECRET_ACCESS_KEY \
-e AWS_SESSION_TOKEN=$AWS_SESSION_TOKEN \
vol-app-api-1 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember we have a CLI container for CLI jobs. This container uses the image that's used in AWS batch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see... Okay, I'll try and get it working with that.

/bin/sh -c "HOME=/tmp php -d memory_limit=1024M /var/www/html/vendor/bin/laminas --container=config/container-cli.php list"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need HOME, or a memory limit setting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at it, memory setting no as default is 2GB so thats okay. As for HOME, I believe that was done on the old environments because we force to be a /tmp dir (cleaned after reboots etc...). Some commands generate content.

This was largley just a tweak of the old way we used to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay, I'd probably say it's best we remove both as it's not typical of the command we run on infrastructure. Your call.

```