hackoregon / backend-examplar-2018

an example dockerized geo-aware django backend
MIT License
4 stars 5 forks source link

fixes for issue #37 and #40 #39

Closed znmeb closed 6 years ago

znmeb commented 6 years ago

Here goes - I added @nam20485 to the contributors too.

MikeTheCanuck commented 6 years ago

I wove in the edits to my copy of the repo but I'm still getting stuck after the containers are built and it's trying to connect them together...

WARNING: Image for service api_development was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
Creating backendexamplar2018_db_development_1 ... done
Creating backendexamplar2018_api_development_1 ... done
Attaching to backendexamplar2018_db_development_1, backendexamplar2018_api_development_1
db_development_1   | The files belonging to this database system will be owned by user "postgres".
db_development_1   | This user must also own the server process.
db_development_1   | 
db_development_1   | The database cluster will be initialized with locale "en_US.utf8".
db_development_1   | The default database encoding has accordingly been set to "UTF8".
db_development_1   | The default text search configuration will be set to "english".
db_development_1   | 
db_development_1   | Data page checksums are disabled.
db_development_1   | 
db_development_1   | fixing permissions on existing directory /var/lib/postgresql/data ... ok
db_development_1   | creating subdirectories ... ok
db_development_1   | selecting default max_connections ... 100
db_development_1   | selecting default shared_buffers ... 128MB
db_development_1   | selecting dynamic shared memory implementation ... posix
db_development_1   | creating configuration files ... ok
api_development_1  | psql: could not connect to server: Connection refused
api_development_1  |    Is the server running on host "db_development" (172.21.0.2) and accepting
api_development_1  |    TCP/IP connections on port 5432?
api_development_1  | Postgres is unavailable - sleeping
...

This is the same initial error I've been seeing on all my attempts today. I wish I could help, but I'm still rusty on how all the pieces of these things fit together, so I'm a bit stumped.

znmeb commented 6 years ago

@MikeTheCanuck

  1. Stop all running containers
  2. Remove the db_development and api_development images
  3. Re-run the "create", "build" and "start" scripts.

Nathan had to do all that to get it running after the fixes. I think we may need to put the image remove back in the "create" script, and a "stop all containers" before it.

MikeTheCanuck commented 6 years ago

OK so here's what I did:

Docker rm $(docker ps -a -q)
Docker rmi $(docker images -q)

Then when I ran ./bin/create-api-project.sh, it terminated at the end with this:

WARNING: Image for service api_development was built because it did not already exist. To rebuild this image you must use `docker-compose build` or `docker-compose up --build`.
CommandError: /code/disaster/wsgi.py already exists, overlaying a project or app into an existing directory won't replace conflicting files
Traceback (most recent call last):
  File "manage.py", line 15, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.6/site-packages/django/core/management/__init__.py", line 371, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.6/site-packages/django/core/management/__init__.py", line 347, in execute
    django.setup()
  File "/usr/local/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/local/lib/python3.6/site-packages/django/apps/registry.py", line 89, in populate
    app_config = AppConfig.create(entry)
  File "/usr/local/lib/python3.6/site-packages/django/apps/config.py", line 90, in create
    module = import_module(entry)
  File "/usr/local/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 953, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'api'

I'm stuck, and it's past bedtime. Thanks for the help.

znmeb commented 6 years ago

If you're trying it with the sample it may not work. The first time it runs, it deletes the database backup. I think for the sample you have to start with bin/build.sh -d. It's my bedtime too.

bhgrant8 commented 6 years ago

@MikeTheCanuck, correct me if i am wrong but are you rerunning the create script in the same repo after creating a new api, with a different name then dead-songs, ie: "disaster"?

So:

Issue 1:

CommandError: /code/disaster/wsgi.py already exists, overlaying a project or app into an existing directory won't replace conflicting files

The remove script has the "dead-songs" name hardcoded into it: https://github.com/hackoregon/backend-examplar-2018/blob/staging/bin/remove-sample.sh#L8

if [ -d ./dead_songs ]; then
    echo "removing django project folder"
    rm -rf dead_songs
fi

when you are running the script a second time in the same directory, the project folder is no longer dead_songs but is not disaster, thus is not removed as part of the create process. The folder/project then already exists when the startproject command tries to create a new project and fails:

https://github.com/hackoregon/backend-examplar-2018/blob/staging/bin/create-api-project.sh#L17

Issue 2:

ModuleNotFoundError: No module named 'api'

Since the create script does name the app itself as "api" for you, this directory did exist and git removed by the remove script:

https://github.com/hackoregon/backend-examplar-2018/blob/staging/bin/remove-sample.sh#L13

if [ -d ./api ]; then
    echo "removing api app folder"
    rm -rf api
fi

Thus it doesn't exist when the ./manage.py command tries to run.

You might be able to fix this with adding an additional check for:

if [ -d ./$PROJECT_NAME ]; then
    echo "removing django project folder"
    rm -rf $PROJECT_NAME
fi

in the remove script.

Otherwise I would suggest we document a suggestion of "Quickstart is only designed to remove the sample application. If you need to restart an api you began, you should clone the examplar from scratch."

nam20485 commented 6 years ago

@MikeTheCanuck In my testing with @znmeb last evening, I found that if you run create-project.sh again (after having run it previously to generate the quickstart app), you will encounter the errors you report above, UNLESS you rm rf the 'api' and '' directories first. The procedure I used successfully several times was:

docker rmi db_development api_development -f sudo rm -rf ./api rm -rf

Once those three steps are done, create-project.sh would complete successfully regardless of whether I had run it previously.

nam20485 commented 6 years ago

I can identify five things that blocked me from successful completion in my attempts to (re-)generate the quickstart app:

  1. Not deleting the 'api' and directories

This topic is covered in @MikeTheCanuck's and @BrianHGrant's post directly above.

  1. Not docker rmi'ing the container images

This was as simple as running docker rmi db_development api_development -f

  1. Not knowing that I had to run bin/build.sh -d after bin/create-project.sh but before bin/start-.sh -d

This may have only been due to some type of ephemeral docker stack corruption I encountered after several cycles of attempts, but purging docker state and a reboot fixed this issue. I don't see that having to run bin/build.sh -d is mentioned anywhere in the instructions.

  1. Not knowing the correct values to use for .env variables for my particular copy of the DB restore.

DEVELOPMENT_POSTGRES_USER=postgres DEVELOPMENT_POSTGRES_NAME=disaster DEVELOPMENT_DATABASE_OWNER=postgres

  1. Ed's fix to 1make-dbuser.sh for correct DB_OWNER which has already been addressed in the PR.

Based on my understanding of the system I agree with Brian's recommendation that we need to fix these issues encountered during re-generation, or mention that it should not be used after it has been run once in the same working copy.

nam20485 commented 6 years ago

Did I inadvertently close this issue? It says I closed it....

znmeb commented 6 years ago

@bhgrant8 @kiniadit @MikeTheCanuck @adpeters I'm thinking I should put the image removes back in the create script - that would eliminate at least one of the stop points here.

BrianHGrant commented 6 years ago

Yeah, as it seems to cause an issue, let's add it back. As folks should not have to rerun too many times, should not be too much of a time hit.

On Tue, Apr 24, 2018, 12:29 PM M. Edward (Ed) Borasky < notifications@github.com> wrote:

@bhgrant8 https://github.com/bhgrant8 @kiniadit https://github.com/kiniadit @MikeTheCanuck https://github.com/MikeTheCanuck @adpeters https://github.com/adpeters I'm thinking I should put the image removes back in the create script - that would eliminate at least one of the stop points here.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hackoregon/backend-examplar-2018/pull/39#issuecomment-384051934, or mute the thread https://github.com/notifications/unsubscribe-auth/ARHAUVb_VblDXMcIm6mH361crMOOpEtyks5tr30wgaJpZM4Tg-NM .

znmeb commented 6 years ago

On Linux, you end up with root-owned files in your repo after you run bin/start.sh -d. See issue #40 for the details. The most recent commit fixes that.

bhgrant8 commented 6 years ago

So it looks like we still need to address a few issues before approving/merging:

Not deleting the 'api' and directories

I think the check on the project directory folder, should resolve but haven't had a chance to test yet

Not knowing that I had to run bin/build.sh -d after bin/create-project.sh but before bin/start-.sh -d

This should have been in step 7, looks like it was dropped in copying and pasting instructions over from our team repo, but yeah we should include

Not knowing the correct values to use for .env variables for my particular copy of the DB restore.

We do mention updating these in step 2 I believe, more then happy to look over docs PR requests to make things clearer if needed.

nam20485 commented 6 years ago

I can perform the two documentation tasks. I already have a few minor key changes in a fork for explaining the .env variables that should be used, and I can go find and put back the build.sh instruction step. (I don't know if you are supposed to run it every time, but I had absolutely 0 clue that it was a thing and that I was missing running that step, until Ed pointed it out to me).

What team repo do you say it was copied from?

nam20485 commented 6 years ago

@bhgrant8 If you can tackle the first problem then I think we will have addressed each of the issues that we have found to this point. Once we have something with all the fixes applied, I can test it out again from the beginning in a blank environment.

znmeb commented 6 years ago

@nam20485 The original was https://github.com/hackoregon/transportation-system-backend.

MikeTheCanuck commented 6 years ago

I will also do a complete run-through once these changes are committed.

znmeb commented 6 years ago

Do we have any other teams ready to build an API?

adpeters commented 6 years ago

@znmeb I've adapted a version of this exemplar for housing. Working with @MikeTheCanuck on the devops side of pushing it to production.

znmeb commented 6 years ago

@adpeters Thanks! I want to make sure we're finding as many issues as possible now. I have a couple of upgrades I want to do (#15, #30) that I'm holding back till we're sure everything is working and the documentation is clear.