Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures #1809

aaronkanzer · 2024-01-11T20:16:13Z

Relates to:

• lincbrain#27

Builds upon:

Cc @kabilar

• lincbrain#26

Cc @yarikoptic @waxlamp

• #1722

Purpose:

For the LINC project -- @kabilar and I have been trying to improve the development process. This PR introduces a simple bash script for local setup, hot-reloading on the frontend, and improved fixtures instantiation.

For the fixtures improvements:

• A minor modification is made to ensure that a User object is created if not found -- this sometimes passed; however, sometimes it did not due to the presence of past data in Postgres being present (especially when switching back-and-forth from linc-archive to dandi-archive) This addition is fairly benign in my opinion

• Extending from #1722, it was observed that validation failed initially upon the creation of an AssetBlob, as with the SimpleUploadedFile as a non-zarr asset, validation fails without a sha_256 initially provided. See the error message below that results from prior these changes

[20:14:42] INFO     Found AssetBlob c000611b-2d6c-4931-b0ec-44855ec1d379                                                                                        __init__.py:21
           INFO     Validating asset metadata for asset 1                                                                                                       __init__.py:40
           INFO     Error while validating asset 1 [{'loc': ('digest',), 'msg': 'A non-zarr asset must have a sha2_256.', 'type': 'value_error'}]               __init__.py:57
           INFO     Validating dandiset metadata for version 1                                                                                                 __init__.py:125
           INFO     Successfully validated version 1

… fixtures

aaronkanzer · 2024-01-11T20:23:59Z

web/package.json

@@ -2,7 +2,7 @@
  "name": "dandi-archive",
  "version": "0.0.0",
  "scripts": {
-    "dev": "vite",
+    "dev": "vite --host 0.0.0.0",


--host 0.0.0.0 is explicitly required here for Docker to bind to localhost appropriately (for local dev)

aaronkanzer · 2024-01-11T21:03:18Z

Cc @mvandenburgh -- just tagging you here to get your thoughts/opinions -- this setup is just something we have found convenience with for the linc-archive fork, but if you have future plans/better ideas, let us know. I noticed all of your frontend improvements regarding vite builds in the end of 2023 😃

yarikoptic · 2024-01-25T15:36:10Z

I know that it was not reviewed yet but conflicts already came up so might be good to rebase @aaronkanzer

yarikoptic · 2024-01-25T15:37:11Z

.gitignore

@@ -123,4 +123,5 @@ dmypy.json
 # End of https://www.gitignore.io/api/django

 # Editor settings
-.vscode 
+.vscode
+.idea/


probably would be good to keep them sorted (since no order effects unless we start adding some !) and that might help to avoid conflicts in some cases at least (in comparison to just adding at the end all the time)

yarikoptic · 2024-01-25T15:42:52Z

admin_dev_startup.sh

+# Happy developing!
+
+if [ $# -lt 2 ]; then
+  echo "Usage: $0 <image_name> <[email protected]>"


Please checkout #1828 . If agree -- may be

make email optional here and if not specified - use that git config user.email

in instructions above - do not make users specify it.

yarikoptic · 2024-01-25T15:45:02Z

DEVELOPMENT.md

-2. Access the site, starting at http://localhost:8000/admin/
-3. When finished, use `Ctrl+C`
+1. Ensure you have installed Docker on your local machine
+2. Run `./admin_dev_startup.sh <fun-image-name-you-can-pick> <your-email>`.  When prompted, enter an username and password in the command prompt. (If you run into local errors with the script, you may need to run `chmod +x admin_dev_startup.sh` first)


Because these are DEVELOPMENT (not DEPLOYMENT) instructions, I like when I could just copy/paste instructions. What about smth like

Suggested change

2. Run `./admin_dev_startup.sh <fun-image-name-you-can-pick> <your-email>`. When prompted, enter an username and password in the command prompt. (If you run into local errors with the script, you may need to run `chmod +x admin_dev_startup.sh` first)

2. Run `./admin_dev_startup.sh EXAMPLE [email protected]` (replace arguments with more appropriate if desired). When prompted, enter an username and password in the command prompt.

I also removed chmod hint -- must not be necessary if you give executable bit to that file in git.

yarikoptic · 2024-01-25T15:55:05Z

admin_dev_startup.sh

@@ -0,0 +1,31 @@
+#!/bin/bash


chmod +x admin_dev_startup.sh; git add admin_dev_startup.sh; git commit -m 'Make script executable' admin_dev_startup.sh

?

yarikoptic · 2024-01-25T15:56:08Z

admin_dev_startup.sh

+  echo "Usage: $0 <image_name> <[email protected]>"
+  exit 1
+fi
+


Suggested change

# Fail as early as any error or any undefined variable

set -eu

yarikoptic · 2024-01-25T15:57:18Z

admin_dev_startup.sh

+cd web/
+
+# Build Docker image (include the path to the Dockerfile's context)
+docker build -t $image_name -f Dockerfile.dev .


Suggested change

docker build -t $image_name -f Dockerfile.dev .

docker build -t "$image_name" -f Dockerfile.dev .

run shellcheck on this script please to not give surprises to people e.g. with spaces in their paths

aaronkanzer · 2024-01-25T16:23:22Z

@yarikoptic thanks for the review here -- I'm navigating over in the Terraform land, so I might not get back to this for a few days -- many thanks in the meantime though

yarikoptic · 2024-01-26T01:47:54Z

since there seems to be no other shell script in this repo I would not introduce shellcheck workflow here, but here is the script with which I would have done it: https://github.com/yarikoptic/improveit/blob/main/shellcheckit ;-)

mvandenburgh · 2024-01-29T15:34:57Z

DEVELOPMENT.md

-3. When finished, use `Ctrl+C`
+1. Ensure you have installed Docker on your local machine
+2. Run `./admin_dev_startup.sh <fun-image-name-you-can-pick> <your-email>`.  When prompted, enter an username and password in the command prompt. (If you run into local errors with the script, you may need to run `chmod +x admin_dev_startup.sh` first)
+3. If not routed to `localhost:8000` upon clicking `LOG IN WITH GITHUB` (If routed appropriately, just log in), navigate to `localhost:8000/admin`, and log in with the username and password you used in Step #2.  Under the `User` section, select the username and change the `Status` from `Incomplete` to `Approved`.


This workaround is actually needed due to a bug, which will be solved by #1113. Once that's merged, this won't be needed.

mvandenburgh · 2024-01-29T15:40:18Z

admin_dev_startup.sh

+# Run the Docker container for frontend in background
+docker run -d -v "$(pwd):/usr/src/app" -v /usr/src/app/node_modules -p 8085:8085 -e CHOKIDAR_USEPOLLING=true $image_name
+
+cd ..
+
+# Run Docker Compose commands for backend
+docker compose run --rm django ./manage.py migrate
+docker compose run --rm django ./manage.py createcachetable
+docker compose run --rm django ./manage.py createsuperuser
+docker compose run --rm django ./manage.py create_dev_dandiset --owner $email
+
+# Bring backend application to life!
+docker compose up


Docker-izing the front end application is nice, I think that's definitely a worthwhile change to make. But, I think adding it as another service in the docker-compose.override.yml would be a better place for it. Starting it with docker run means that the user then has to manually stop it, and can't rely on docker compose down/stop to stop the container.

mvandenburgh · 2024-01-29T15:44:38Z

dandiapi/api/management/commands/create_dev_dandiset.py

+@click.option('--first_name', default='Randi The Admin')
+@click.option('--last_name', default='Dandi')
+def create_dev_dandiset(name: str, email: str, first_name: str, last_name: str):
+    owner, is_created = User.objects.get_or_create(email=email)
+    if not is_created:
+        owner.first_name = first_name
+        owner.last_name = last_name
+        owner.save()


The intent of this script is for it to be run after manage.py createsuperuser. In the spirit of keeping the original scope of the script (i.e., creating a single dandiset with an asset, not any users), my preference would be to leave this as-is and not add additional logic for creating users.

mvandenburgh · 2024-01-29T15:51:24Z

admin_dev_startup.sh

+docker compose run --rm django ./manage.py create_dev_dandiset --owner $email
+
+# Bring backend application to life!
+docker compose up


This script is nice, but I disagree that we should make it the default way to start a dev environment for a couple reasons -

It abstracts away too much. For example, I think running DB migrations should be an intentional and explicit action on the part of the developer. There are some cases in development where one may want to hold off on running existing migrations, or only run a subset of migrations.

I think it's fairly common to want to run the frontend server natively and all the backend services in Docker, particularly when doing frontend-only development.

mvandenburgh · 2024-01-29T15:58:36Z

dandiapi/api/management/commands/create_dev_dandiset.py

+        # Since the SimpleUploadedFile is non-zarr asset, validation fails
+        # without a sha2_256 initially provided.
+        sha256_hash = hashlib.sha256(file_content).hexdigest()


Can confirm this fixes the validation failure for me locally 👍

yarikoptic · 2024-09-09T22:00:37Z

@mvandenburgh thanks for the review! I think it is still worth reincarnating this PR and a developer helper, WDYT @aaronkanzer , would you be able to resolve conflicts and reflect on @mvandenburgh comments?

aaronkanzer · 2024-09-16T17:36:47Z

Just re-visiting now -- sorry for the delay, thank you @mvandenburgh @waxlamp and @yarikoptic for the review.

I think I'm going to close this PR and not move forward to move (I'll just resort to using this as a local utility). I'd rather not make it a configurable script that is merged (seems like an increase in cognitive overhead if this is not unanimously wanted by other folks)

Aaron Kanzer added 2 commits January 11, 2024 14:38

Streamline local dev process with hot reloading, docker, and resolved…

48dde05

… fixtures

make command less error-prone

ccfd739

aaronkanzer mentioned this pull request Jan 11, 2024

Resolve broken fixtures in dev dataset script for local Dandi #1722

Closed

aaronkanzer changed the title ~~Draft: Streamline local dev process with hot reloading, docker, and resolved fixtures~~ Streamline local dev process with hot reloading, Dockerfile updates, and resolved fixtures Jan 11, 2024

aaronkanzer mentioned this pull request Jan 11, 2024

Create dockerized development environment for LINC and DANDI lincbrain/linc-archive#27

Open

2 tasks

aaronkanzer commented Jan 11, 2024

View reviewed changes

Aaron Kanzer added 2 commits January 11, 2024 15:26

bump

14cb1a3

bump readme for code bases in different states

25e01d6

aaronkanzer changed the title ~~Streamline local dev process with hot reloading, Dockerfile updates, and resolved fixtures~~ Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures Jan 11, 2024

kabilar mentioned this pull request Jan 17, 2024

Create simple startup script for admin development on linc-brain lincbrain/linc-archive#26

Merged

include kabi review with minor adjustment to gitignore

31789a5

yarikoptic requested changes Jan 25, 2024

View reviewed changes

mvandenburgh reviewed Jan 29, 2024

View reviewed changes

waxlamp self-assigned this Sep 10, 2024

aaronkanzer closed this Sep 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures #1809

Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures #1809

aaronkanzer commented Jan 11, 2024 •

edited

Loading

aaronkanzer Jan 11, 2024

aaronkanzer commented Jan 11, 2024 •

edited

Loading

yarikoptic commented Jan 25, 2024

yarikoptic Jan 25, 2024

yarikoptic Jan 25, 2024

yarikoptic Jan 25, 2024

yarikoptic Jan 25, 2024

yarikoptic Jan 25, 2024

yarikoptic Jan 25, 2024

aaronkanzer commented Jan 25, 2024

yarikoptic commented Jan 26, 2024

mvandenburgh Jan 29, 2024

mvandenburgh Jan 29, 2024

mvandenburgh Jan 29, 2024

mvandenburgh Jan 29, 2024

mvandenburgh Jan 29, 2024

yarikoptic commented Sep 9, 2024

aaronkanzer commented Sep 16, 2024

	2. Run `./admin_dev_startup.sh <fun-image-name-you-can-pick> <your-email>`. When prompted, enter an username and password in the command prompt. (If you run into local errors with the script, you may need to run `chmod +x admin_dev_startup.sh` first)
	2. Run `./admin_dev_startup.sh EXAMPLE [email protected]` (replace arguments with more appropriate if desired). When prompted, enter an username and password in the command prompt.



	# Fail as early as any error or any undefined variable
	set -eu

	docker build -t $image_name -f Dockerfile.dev .
	docker build -t "$image_name" -f Dockerfile.dev .

Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures #1809

Streamline local dev process with hot reloading, Dockerfile.dev updates, and resolved fixtures #1809

Conversation

aaronkanzer commented Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

aaronkanzer commented Jan 11, 2024 • edited Loading

yarikoptic commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronkanzer commented Jan 25, 2024

yarikoptic commented Jan 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarikoptic commented Sep 9, 2024

aaronkanzer commented Sep 16, 2024

aaronkanzer commented Jan 11, 2024 •

edited

Loading

aaronkanzer commented Jan 11, 2024 •

edited

Loading