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

Configure docker for development #465

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

MrQubo
Copy link
Member

@MrQubo MrQubo commented May 6, 2021

Configured docker image to be usable for development.
Also fixed the image and locked base images versions to prevent future errors.


This change is Reviewable

MrQubo added 3 commits May 7, 2021 01:31
Configured docker image to be usable for development.
Also fixed the image and locked base images versions to prevent future errors.
I've realized it looks poorly with tab width shorter than 5.
@MrQubo
Copy link
Member Author

MrQubo commented May 6, 2021

Something jebło.

@@ -17,6 +17,7 @@ Django-based application to manage registration of people for [scientific summer
- `./manage.py populate_with_test_data` - script to populate the database with data for development

### Run:
- install postgresql
Copy link
Member

Choose a reason for hiding this comment

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

only in production deployments - for development, sqlite is used instead

Copy link
Member Author

Choose a reason for hiding this comment

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

For me pip install -r requirements.txt failed because of missing pg_config executable.

Copy link
Member

Choose a reason for hiding this comment

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

A @krzys-h has said, postgress should not be required for development. requirements.txt file should be adjusted if it is there

Copy link
Member

Choose a reason for hiding this comment

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

You can create a requirements.txt file for production

export DJANGO_SUPERUSER_PASSWORD=admin \
export DJANGO_SUPERUSER_USERNAME=admin
# This will fail if admin user already exists.
python3 manage.py createsuperuser --noinput || true
Copy link
Member

Choose a reason for hiding this comment

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

This is problematic. What if I remove/rename the initial user after creating my own account?

I know this is only for development Docker, but...

Copy link
Member Author

Choose a reason for hiding this comment

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

What problem do you have in mind? Can you elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

It becomes impossible to remove/change username of the default user, as it will get recreated when the container restarts. If you really want automatic user creation, it should only occur once, when the database is first created.

But personally I don't think this is necessary at all... If you want a development setup, call populate_with_test_data after starting the container which creates the admin account too iirc, if you want a production setup, don't use admin as the password...

@@ -17,6 +17,7 @@ Django-based application to manage registration of people for [scientific summer
- `./manage.py populate_with_test_data` - script to populate the database with data for development

### Run:
- install postgresql
Copy link
Member

Choose a reason for hiding this comment

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

A @krzys-h has said, postgress should not be required for development. requirements.txt file should be adjusted if it is there

@@ -17,6 +17,7 @@ Django-based application to manage registration of people for [scientific summer
- `./manage.py populate_with_test_data` - script to populate the database with data for development

### Run:
- install postgresql
Copy link
Member

Choose a reason for hiding this comment

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

You can create a requirements.txt file for production

Dockerfile Show resolved Hide resolved
@@ -0,0 +1,6 @@
coverage==5.5
Copy link
Member

Choose a reason for hiding this comment

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

You probably need to update CI so it properly installs this requirements for running tests

Copy link
Member

Choose a reason for hiding this comment

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

He already did - the tests fail for a different reason, the same fail message is in the other PR which doesn't touch requirements

@MrQubo MrQubo marked this pull request as draft May 8, 2021 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants