fbdevelopercircles / open-source-edu-bot

Open Source Education bot, built by the Developer Circles community.
MIT License
59 stars 79 forks source link

fix: changed DB config #102

Closed vj-codes closed 3 years ago

vj-codes commented 4 years ago

Is your pull request related to a problem? Please describe

Fixed issue #27

Steps done -

Steps remaining-

Checklist

vj-codes commented 4 years ago

@aboullaite went through CI, any idea why is the build failing didn't understand why exactly. Also any resources to add tests?

aboullaite commented 4 years ago

The log says: ERROR: Invalid requirement: 'Flask-SQLAlchemy>=2.4.4,2.4.5' (from line 16 of /requirements.txt)

aboullaite commented 4 years ago

@vj-codes the build is still failing! is the app not complaining at your env ? do you have docker on your machine! You can probably try to run Dockerfile and add any missing dependencies.

vj-codes commented 4 years ago

@aboullaite @elinguiuriel I did not change anything, works completely fine locally. I don't use docker , should the packages mentioned above be added ? Screenshot (50)

elinguiuriel commented 4 years ago

@aboullaite @elinguiuriel I did not change anything, works completely fine locally. I don't use docker , should the packages mentioned above be added ? Screenshot (50)

@vj-codes it's good to know that the code work locally, that does not guarantee that it will work on another computer. Maybe you use your computer for many other things and you already have the dependencies installed, think about those that do not have them already installed. The code is tested and deployed using Docker to mimic installation in a totally new environment, if you pass the docker installation then we can be sure that the code will work merely everywhere.

Just make sure to add the instructions described above in the 2 in Dockerfile and dev.Dockerfile and you may pass the tests

dev.Dockerfile

# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

FROM python:3.7-alpine
LABEL MAINTAINER "Facebook Developers Circles" 

RUN apk add --update --no-cache postgresql-client
RUN apk add --update --no-cache --virtual .tmp-build-deps \
    gcc libc-dev linux-headers postgresql-dev 

RUN apk add --no-cache python3-dev libffi-dev gcc musl-dev make
RUN pip install gunicorn[gevent]

COPY requirements.txt /requirements.txt
RUN pip install -r /requirements.txt

WORKDIR /app
COPY ./src .

RUN pybabel compile -d locales

EXPOSE 5000

CMD gunicorn "fbosbot:create_app()" --worker-class gevent --workers 5 --bind 0.0.0.0:$PORT --max-requests 10000 --timeout 5 --keep-alive 5 --log-level info

Dockerfile

# Copyright (c) Facebook, Inc. and its affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

FROM python:3.7-alpine
LABEL MAINTAINER "Facebook Developers Circles" 

WORKDIR /app

COPY ./src .

RUN apk add --update --no-cache postgresql-client
RUN apk add --update --no-cache --virtual .tmp-build-deps \
    gcc libc-dev linux-headers postgresql-dev 

COPY requirements.txt /requirements.txt
RUN pip install -r /requirements.txt
RUN pybabel compile -d locales
vj-codes commented 4 years ago

@elinguiuriel @aboullaite The build is successful now! What shall be done for tests and I don't exactly understand what is to be done model structure mentioned in the issue.

elinguiuriel commented 4 years ago

@elinguiuriel @aboullaite The build is successful now! What shall be done for tests and I don't exactly understand what is to be done model structure mentioned in the issue.

You only have to test like this when you install a new package. if possible uses docker to not be biased by your environment preinstalled dependencies When you simply modify code, make sure this command pass without error

(venv) $ pytest && flake8

the CI/CD workflows use it to check PR and merge to the master branch

vj-codes commented 4 years ago

@elinguiuriel 5 tests passing one failing Screenshot (52)

aboullaite commented 4 years ago

@vj-codes What's the status of this one ? were you able to add @elinguiuriel changes ?

vj-codes commented 4 years ago

@vj-codes What's the status of this one ? were you able to add @elinguiuriel changes ?

Yes @aboullaite I added it, everything is working good only concern is that one test is failing(shared screen shot above)

Awinja-j commented 3 years ago

@vj-codes you need to set up verify_token in your env and It should work proper.

LGTM @elinguiuriel any more change requests?