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

Adding Dockerfile to application #152

Closed
wants to merge 10 commits into from

Conversation

teezzan
Copy link

@teezzan teezzan commented Oct 2, 2021

This PR helps add a Dockerfile to the project. An Image of the project can easily be created and built for faster deploys.

Develop PR Description

NA

Related Issue

#106


Motivation and Context

It is a little bit hard to set up an environment from scratch. Docker helps!

How Has This Been Manually Tested?

Will be tested locally on my Github codespace console and deployed to check if it works.


Development Checklist:

Documentation:

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • A documentation label has been added to the PR

Testing:

  • My change requires unit tests to be written
    • I have added tests to cover my changes.
  • I have manually tested my changes to the best of my ability

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2021

Codecov Report

Merging #152 (bce4d6b) into dev (186ee86) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #152      +/-   ##
==========================================
+ Coverage   66.22%   66.26%   +0.03%     
==========================================
  Files         106      106              
  Lines        5028     5028              
  Branches      348      348              
==========================================
+ Hits         3330     3332       +2     
+ Misses       1654     1653       -1     
+ Partials       44       43       -1     
Impacted Files Coverage Δ
...es/box__default/auth/tests/test_auth_functional.py 69.53% <0.00%> (+1.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 186ee86...bce4d6b. Read the comment docs.

@teezzan
Copy link
Author

teezzan commented Oct 2, 2021

@Abdur-rahmaanJ Can you give me a list of environment variables that the application needs to run? My plan is to feed the docker image a .env file while starting it. What do you think?

@kfields You can check it out too.

@Abdur-rahmaanJ
Copy link
Member

@teezzan we are currently using instance/config.py. Have you ever heard of instance/ in Flask?

@teezzan
Copy link
Author

teezzan commented Oct 2, 2021

Not at all. Have you any link to it? Maybe how I can integrate it with Docker.

@teezzan
Copy link
Author

teezzan commented Oct 2, 2021

I will probably just allow the user to pass in a .env file or config.py then move it into the newly created /instance folder.

@Abdur-rahmaanJ
Copy link
Member

@teezzan https://flask.palletsprojects.com/en/2.0.x/config/#instance-folders ok that sounds good. See readme, we are creating an instance folder then adding config.py file

Btw are you on the Pallets discord?

@teezzan
Copy link
Author

teezzan commented Oct 2, 2021

@Abdur-rahmaanJ Thank you. Please share with me a link to the Pallets Discord channel.

@Abdur-rahmaanJ
Copy link
Member

https://discord.gg/pallets

Go to #flaskcon-waiting-room

Ping @appinv

And change your name to your real name just for the server as you are joining as a FlaskCon staff

Dockerfile Outdated
RUN mkdir instance
RUN touch instance/config.py
#temporary until i get the docker compose up
RUN echo SQLALCHEMY_DATABASE_URI = '"mysql+pymysql://hB3NLp4kiW:5GBiMgmHyB@remotemysql.com:3306/hB3NLp4kiW"' >> ./instance/config.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this.

Dockerfile Outdated
Comment on lines 12 to 13
COPY dev_requirements.txt /app
RUN pip install -r dev_requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to add dev requirements if the docker image will only be used for deployments?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I think I don't need to add maildev to the services too. Since it is only for deployment. Is that okay?

Copy link
Author

Choose a reason for hiding this comment

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

But it leads to a series of problems. First one is that the application can't find Faker when it starts.
I removed the initialise command and the seeding command because they were raising errors about things that are Dev dependencies not found. What do you say about this?

@kfields
Copy link
Contributor

kfields commented Oct 3, 2021

Nice! Some competition. However I'm getting errors

WARNING: The MYSQLDB_ROOT_PASSWORD variable is not set. Defaulting to a blank string.
WARNING: The MYSQLDB_DATABASE variable is not set. Defaulting to a blank string.
WARNING: The MYSQLDB_LOCAL_PORT variable is not set. Defaulting to a blank string.
WARNING: The MYSQLDB_DOCKER_PORT variable is not set. Defaulting to a blank string.
WARNING: The FLASK_LOCAL_PORT variable is not set. Defaulting to a blank string.
WARNING: The FLASK_DOCKER_PORT variable is not set. Defaulting to a blank string.
ERROR: The Compose file './docker-compose.yml' is invalid because:
services.app.ports contains an invalid type, it should be a number, or an object
services.mysqldb.ports contains an invalid type, it should be a number, or an object

You need to use a .env file or a combination of .env and env_file:

env_file can only set environment variables inside a service container. Variables from env_file cannot be injected into docker-compose.yml itself.

I encountered the same problem, so I quit using env_file. Now that I understand more fully I might use a combination of .env and env_file

@teezzan
Copy link
Author

teezzan commented Oct 3, 2021

@kfields, I had the same issue. I simply stopped using the .env file for now.

@teezzan
Copy link
Author

teezzan commented Oct 3, 2021

@Abdur-rahmaanJ @michaelbukachi The final error that I have is the inability to connect to the SQL server instance. I am trying to debug that as we speak.

@kfields
Copy link
Contributor

kfields commented Oct 3, 2021

Put the dev stuff back in. We can write a production configuration later. It won't even run because it can't find 'faker'

COPY dev_requirements.txt /app
RUN pip install -r dev_requirements.txt

PS: I'll look into your MySQL problem

@Abdur-rahmaanJ
Copy link
Member

@teezzan great!!! waiting for it to finish

@Abdur-rahmaanJ
Copy link
Member

Btw we dont need nodejs for tests.

@teezzan
Copy link
Author

teezzan commented Oct 10, 2021

It was a mistake 😂. I Am still unable to connect to the SQL server instance created by docker compose. Every other thing works. I might need a little bit of help at this point. If you are available for a joint troubleshooting, let me know. @kfields @Abdur-rahmaanJ

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.

5 participants