Page MenuHomePhabricator

[Nix] Add localstack-up
ClosedPublic

Authored by jon on Sep 27 2022, 9:38 PM.
Tags
None
Referenced Files
F3248101: D5245.id17365.diff
Fri, Nov 15, 8:37 AM
F3248087: D5245.id17124.diff
Fri, Nov 15, 8:33 AM
F3248081: D5245.id17393.diff
Fri, Nov 15, 8:31 AM
F3248058: D5245.id17471.diff
Fri, Nov 15, 8:23 AM
F3248054: D5245.id17391.diff
Fri, Nov 15, 8:23 AM
F3247717: D5245.diff
Fri, Nov 15, 6:28 AM
Unknown Object (File)
Wed, Nov 6, 8:54 AM
Unknown Object (File)
Tue, Oct 22, 12:06 PM
Subscribers

Details

Reviewers
abosh
varun
ashoat
atul
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMM8607112867c1: [Nix] Add localstack-up
Summary

Allow for localstack to be easily started through nix

Depends on D5244

Test Plan
nix run .#localstack-up
# should get a message about localstack starting
docker ps # should see localstack in list

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Owners added a reviewer: Restricted Owners Package.Sep 27 2022, 9:38 PM
atul removed a reviewer: atul. atul added 1 blocking reviewer(s): abosh.

Resigning and adding @abosh as blocking. Anticipate you'll get some pushback on shell within writeShellApplication instead of external file

Can we stop writing bash inline in Nix files? This has been discussed numerous times during review, not sure where we landed but it still feels like a code smell

Can we stop writing bash inline in Nix files? This has been discussed numerous times during review, not sure where we landed but it still feels like a code smell

The main thing is that ${localstack} will be interpolated to the nix store location. The "bash" still gets linted by shellcheck, and any nix command leveraging this would fail to build it.

The rendered file will look something like:

$ nix build .#localstack-up
warning: Using saved setting for 'extra-trusted-public-keys = comm.cachix.org-1:70RF31rkmCEhQ9HrXA2uXcpqQKGcUK3TxLJdgcUCaA4=' from ~/.local/share/nix/trusted-settings.json.
warning: Using saved setting for 'extra-trusted-substituters = https://comm.cachix.org' from ~/.local/share/nix/trusted-settings.json.
(nix-shell) [10:04:27] jon@jon-desktop /home/jon/comm/comm (jonringer/localstack)
$ cat ./result/bin/localstack-up
#!/nix/store/p7bpdnxqd3i5hwm92mrscf7mvxk66404-bash-5.1-p16/bin/bash
set -o errexit
set -o nounset
set -o pipefail

# Avoid localstack attempt to write in the nix store
XDG_DATA_HOME=${XDG_DATA_HOME:-$HOME/.local/share}
export FILESYSTEM_ROOT=${XDG_DATA_HOME}/localstack/filesystem

# Since docker is installed outside of nix, need to ensure that it was
# installed impurely
if ! command -v docker > /dev/null; then
  echo "Please install docker in order to use localstack" >&2
  exit 1
fi

# The 'localstack status' command will poll foever if you have a newer
# docker cli, so instead use docker ps + grep to determine running container
if ! docker ps | grep localstack &> /dev/null; then
  echo "Starting localstack..." >&2
  /nix/store/6qjayc18ircvp764ccdapzqnjji8khfy-python3.10-localstack-1.0.4/bin/localstack start \
    --detached \
    --docker \
    --no-banner > /dev/null
else
  echo "localstack is already running, skipping localstack initialization"
fi

# Explicitly exit this script so the parent shell can determine
# when it's safe to return control of terminal to user
exit 0

Can't localstack be passed in to the shell script? Then it could be a standalone script. You can either use environmental variables or pass as a parameter via command-line. What downsides do you see to that approach? Can we adopt that approach for all shell scripts going forward, so we're not defining shell logic inline in a .nix file?

What downsides do you see to that approach?

Depends on whether you want the script to always work. Refactoring everything into a script, you would always need to do everything within nix develop. Where as, you can also do nix run .#localstack-up here.

I would say doing it this way is the more correct nix way of doing it, but I could probably refactor it to be a script with assumptions, and a nix command which fulfills those assumptions.

ashoat requested changes to this revision.Oct 4 2022, 6:26 PM

In the past, when you've factored out Nix scripts following feedback on diffs, how did you do that? Can we do the same thing here?

This revision now requires changes to proceed.Oct 4 2022, 6:26 PM

Refactor into script + nix wrapper

Thank you!!

scripts/localstack_up.sh
36 ↗(On Diff #17365)

Can we cut this extraneous newline at the bottom?

@abosh is busy with classwork

ashoat requested changes to this revision.Oct 6 2022, 5:50 AM

(Forgot that @jon prefers Request Changes if any changes are requested. See my comment inline about extraneous newline)

This revision now requires changes to proceed.Oct 6 2022, 5:50 AM
atul added a subscriber: atul.
atul@atuls-MacBook-Pro comm % nix run .#localstack-up
Starting localstack...
atul@atuls-MacBook-Pro comm % docker ps
CONTAINER ID   IMAGE                   COMMAND                  CREATED         STATUS                          PORTS                                                                                                                          NAMES
29a88313ba43   localstack/localstack   "docker-entrypoint.sh"   7 seconds ago   Up 5 seconds                    127.0.0.1:4510-4559->4510-4559/tcp, 127.0.0.1:4566->4566/tcp, 127.0.0.1:4571->4571/tcp, 127.0.0.1:12121->12121/tcp, 5678/tcp   localstack_main
66e5c834328a   8b8a9d2f4301            "docker-entrypoint.s…"   3 months ago    Restarting (2) 56 seconds ago                                                                                                                                  keyserver-node-1
b9af4b1e8f69   mysql:5.7.37-debian     "docker-entrypoint.s…"   3 months ago    Up 19 hours                     3306/tcp, 33060/tcp                                                                                                            keyserver-database-1
1310a62370a0   redis:6.2.6-bullseye    "docker-entrypoint.s…"   3 months ago    Up 19 hours                     6379/tcp                                                                                                                       keyserver-cache-1
atul@atuls-MacBook-Pro comm %
This revision is now accepted and ready to land.Oct 7 2022, 11:09 AM
This revision was automatically updated to reflect the committed changes.