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

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
17 changes: 5 additions & 12 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@
## Develop with Docker (recommended quickstart)
This is the simplest configuration for developers to start with.

### Initial Setup
1. Run `docker-compose run --rm django ./manage.py migrate`
2. Run `docker-compose run --rm django ./manage.py createcachetable`
3. Run `docker-compose run --rm django ./manage.py createsuperuser`
and follow the prompts to create your own user.
Set your username to your email to ensure parity with how GitHub logins work.
4. Run `docker-compose run --rm django ./manage.py create_dev_dandiset --owner your.email@email.com`
to create a dummy dandiset to start working with.
## Instructions for local development with front-end hot reloading

### Run Application
1. Run `docker-compose up`
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)
Copy link
Member

Choose a reason for hiding this comment

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

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 myemail@example.com` (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.

3. 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`.
4. Navigate to `localhost:8085` and select `LOG IN WITH GITHUB`.

### Application Maintenance
Occasionally, new package dependencies or schema changes will necessitate
Expand Down
31 changes: 31 additions & 0 deletions admin_dev_startup.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

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

?


# This script can be used to launch both the frontend and backend with ease locally
# Happy developing!

if [ $# -lt 2 ]; then
echo "Usage: $0 <image_name> <your.email@email.com>"
Copy link
Member

Choose a reason for hiding this comment

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

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.

exit 1
fi

Copy link
Member

Choose a reason for hiding this comment

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

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

image_name=$1
email=$2

cd web/

# Build Docker image (include the path to the Dockerfile's context)
docker build -t $image_name -f Dockerfile.dev .
Copy link
Member

Choose a reason for hiding this comment

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

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


# 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
24 changes: 17 additions & 7 deletions dandiapi/api/management/commands/create_dev_dandiset.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import hashlib
from uuid import uuid4

from django.conf import settings
Expand All @@ -15,8 +16,14 @@
@click.command()
@click.option('--name', default='Development Dandiset')
@click.option('--owner', 'email', required=True, help='The email address of the owner')
def create_dev_dandiset(*, name: str, email: str):
owner = User.objects.get(email=email)
@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()
Comment on lines +19 to +26
Copy link
Member

Choose a reason for hiding this comment

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

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.


version_metadata = {
'description': 'An informative description',
Expand All @@ -26,16 +33,19 @@ def create_dev_dandiset(*, name: str, email: str):
user=owner, embargo=False, version_name=name, version_metadata=version_metadata
)

uploaded_file = SimpleUploadedFile(name='foo/bar.txt', content=b'A' * 20)
file_size = 20
file_content = b'A' * file_size
uploaded_file = SimpleUploadedFile(name='foo/bar.txt', content=file_content)
etag = '76d36e98f312e98ff908c8c82c8dd623-0'

try:
asset_blob = AssetBlob.objects.get(etag=etag)
except AssetBlob.DoesNotExist:
# Since the SimpleUploadedFile is non-zarr asset, validation fails
# without a sha2_256 initially provided.
sha256_hash = hashlib.sha256(file_content).hexdigest()
Comment on lines +44 to +46
Copy link
Member

Choose a reason for hiding this comment

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

Can confirm this fixes the validation failure for me locally 👍

asset_blob = AssetBlob(
blob_id=uuid4(),
blob=uploaded_file,
etag=etag,
size=20,
blob_id=uuid4(), blob=uploaded_file, etag=etag, size=file_size, sha256=sha256_hash
)
asset_blob.save()
asset_metadata = {
Expand Down
18 changes: 18 additions & 0 deletions web/Dockerfile.dev
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
FROM node:20

# Set the working directory
WORKDIR /usr/src/app

# Copy package.json and install dependencies
COPY package.json yarn.lock ./
RUN yarn install --production=false

# Copy the rest of your application's code
COPY . .

# Expose the port the app runs on
EXPOSE 8085

# Start the application
CMD ["yarn", "run", "dev"]

2 changes: 1 addition & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "dandi-archive",
"version": "0.0.0",
"scripts": {
"dev": "vite",
"dev": "vite --host 0.0.0.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

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

"build": "vite build",
"preview": "vite preview --port 4173",
"type-check": "vue-tsc --noEmit",
Expand Down