CodeWithAloha / uipa

Helping submit, track, and share public records requests in Hawaii
http://uipa.org
MIT License
10 stars 6 forks source link

V2 #83

Closed sthapa closed 3 months ago

sthapa commented 8 months ago

Add updates to allow docker-compose with dev configs to work

tyliec commented 8 months ago

I wasn't able to get this PR working locally as is, is there a permission issue as well somewhere?:

➜  uipa git:(v2) ✗ docker-compose -f docker-compose.local.yml up --force-recreate
[+] Running 1/1
 ✘ app Error                                                                                                                                                          1.9s 
Error response from daemon: pull access denied for sthapa/uipa_backend, repository does not exist or may require 'docker login': denied: requested access to the resource is denied
russtoku commented 8 months ago

I get the same error:

$ docker manifest inspect ssthapa/uipa_backend:0.9
no such manifest: docker.io/ssthapa/uipa_backend:0.9

However, an earlier version of ssthapa/uipa_backend does exist.

$ docker manifest inspect ssthapa/uipa_backend:0.8
{
    "schemaVersion": 2,
    "mediaType": "application/vnd.oci.image.manifest.v1+json",
    "config": {
        "mediaType": "application/vnd.oci.image.config.v1+json",
        "digest": "sha256:c92a2bac4d2c7b6428986b281b07a5be2989995068848b60535db73d02e5fa74",
        "size": 12096
    },
    "layers": [
...
    ],
    "annotations": {
        "org.opencontainers.image.base.digest": "sha256:fde9b5a36f85656f9df800b174af67d49a2189b0e7fdc8c0c5e4e1416f403a01",
        "org.opencontainers.image.base.name": "docker.io/ssthapa/django5:0.1"
    }
}
russtoku commented 8 months ago

Hmm...things seem a bit complicated and opaque. Is there a need to use a special image (ssthapa/uipa_backend)?

sthapa commented 8 months ago

@tyliec you'll need to login to dockerhub to pull the image I think. docker made it more difficult to use without a login.

@russtoku we should at the baseline provide a baseline image that works. If devs can change the image reference and use a locally built version but it's a bit more cumbersome and may take 5-10 minutes to build initially. Also with the pre-built image, it makes building a local image much faster.

russtoku commented 8 months ago

After running docker-compose -f docker-compose.local.yml up, I get this when I visit http://localhost:8000/:

pr-83-django-error

I have to disagree with the use of poetry and the removal of requirements.txt for the use case of a person wanting to help with the backend and the frontend and trying to get a local working environment. If they can't run the Django dev server locally and must use Docker containers, what do they do? They won't have the appropriate experience and skills to debug things to get it working. Also, once they get a working Django dev server in a container, would they know how to go about making changes and presenting those changes in a pull request?

While Docker containers can be a way to quickly get up to speed, not everyone wants to and is able to use Docker locally. While it may be OK to use poetry in non-package mode to install the dependencies, it's probably beyond the reach of new-comers who lack the experience with it.

I'm OK with adding docker-compose.local.yml and pyproject.toml at the top-level but not with forcing people to use poetry to install dependencies.

sthapa commented 8 months ago

@russtoku What were you doing when you got that error? Can you try it with the 0.9 image and the updated docker compose file.

I'm okay with sticking with the requirements.txt file if we have a good setup and documentation that lets people use it but with all due respect, the current requirements workflow is broken. All the code with aloha devs have had to spend multiple hours getting things running with it with varying degrees of success. Yen spent 2 hours on it with our guidance and had to stop due to an error that none of us have seen. And that's the second or third session that we've tried to help her get things running. I think the current situation is a huge blocker for any new devs. If there's a documentation or code fix that'll resolve this, lets get that in and use the requirements.txt but unless we have a reliable way of onboarding new devs, it's a huge problem for us.

Using containers takes a little change in your workflow but it's possible to work locally using containers without issues. PyCharm and VScode both allow for editing and hotloading code into a container and I've personally debugged multithreaded c++ servers running on containers with the ability to step through functions, update variables and even disassemble code. You can edit code locally and see changes run on a container with fairly minimal chantges to your workflow.

russtoku commented 8 months ago

After running docker-compose -f docker-compose.local.yml up --force-recreate, I get this error when I visit http://localhost:8000/:

uipa-docker-error-pr-83

This is a different error from my previous attempt. In both cases, after cloning the repo and checking out you PR/83 branch, I run the docker-compose command to start up the containers.

The first try used ssthapa/uipa_backend:0.8 because there was no access to 0.9. The second try used ssthapa/uipa_backend:0.9.

This second try indicates that the migrations weren't run to initialze the database before the Django dev server was started. This would suggest that the sequencing of steps are not quite there yet. Here's the output from the dev server container:

app-1            | django-configurations version 2.5.1, using configuration Dev                        
app-1            | Performing system checks...                                                         
app-1            |                                                                                     
app-1            | /root/.cache/pypoetry/virtualenvs/uipa-JzPo6xSy-py3.12/lib/python3.12/site-packages/weasyprint/text/fonts.py:65: UserWarning: No fonts configured in FontConfig. Expect ugly output.       
app-1            |   warn('No fonts configured in FontConfig. Expect ugly output.')                    
app-1            | System check identified no issues (0 silenced).                                     
app-1            |                                                                                     
app-1            | You have 326 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): accesstoken, account, admin, auth, bounce, campaign, comments, contenttypes, django_celery_beat, django_comments, document, easy_thumbnails, filingcabinet, flatpages, foirequest, foirequestfollower, foisite, frontpage, georegion, guide, letter, mfa, oauth2_provider, organization, problem, proof, publicbody, sessions, sites, taggit, team, upload.                                  
app-1            | Run 'python manage.py migrate' to apply them.                                       
app-1            | April 03, 2024 - 23:14:11                                                           
app-1            | Django version 5.0.3, using settings 'froide.settings'                              
app-1            | Starting development server at http://0.0.0.0:8000/                                 
app-1            | Quit the server with CONTROL-C.
russtoku commented 8 months ago

I'm personally OK with whatever is decided; even removing requirements.txt. I'm trying to advocate for a better developer experience so that anyone is welcome to try their hand at improving UIPA.

sthapa commented 7 months ago

@russtoku The error you're getting is probably due to the migrations not being applied. The migrations get applied before the django container runs so you shouldn't see that. Can you post the full output of the docker-compose up invocation?

russtoku commented 7 months ago

Sorry, I don’t have access to my Macbook Air for three weeks.

russtoku commented 6 months ago

With commit 3faa89f, this is what I get when trying to start the containers:

(py310) uipa (83)$ git log --oneline -n 5
3faa89f9 (HEAD -> pr/83, origin/v2) WIP getting uipa running in container
62e1b149 Point to new image version, clean up env vars
87a68578 Fix image reference and cleanups
d3c09cbf Merge branch 'main' into v2
f32a335a Fix elasticsearch references

(py310) uipa (83)$ docker-compose -f docker-compose.local.yml up --force-recreate
WARN[0000] /Users/russ/Projects/Code_With_Aloha/pr-83/uipa/docker-compose.local.yml: `version` is obsolete 
[+] Running 1/1
 ✘ app Error manifest for ssthapa/uipa_backend:0.10 not foun...            6.2s 
Error response from daemon: manifest for ssthapa/uipa_backend:0.10 not found: manifest unknown: manifest unknown