-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Issue#974 in talawa-api (Docker for Easy Local Devlopment) #1418
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
Conversation
Please fix the first comment so that each issue listed automatically closes. The PR_GUIDELINES.md file has details. Please also fill in the template for the pull request as completely as you can. It greatly helps others searching for code changes in future and helps others understand the summarized rationale for the work you have done |
Codecov Report
@@ Coverage Diff @@
## develop #1418 +/- ##
===========================================
- Coverage 98.17% 97.42% -0.76%
===========================================
Files 184 203 +19
Lines 10767 12060 +1293
Branches 835 945 +110
===========================================
+ Hits 10571 11749 +1178
- Misses 186 296 +110
- Partials 10 15 +5
... and 1 file with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
@palisadoes Updated the first comment with summary and proper issue number according to PR_GUIDELINES.md |
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersWhen your PR has been assigned reviewers contact them to get your code reviewed and approved via:
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
@palisadoes thank you for review, added the changes suggested by you |
@EshaanAgg @xoldyckk Please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see you setting you up the other environment variables which are asked by the setup script anywhere in the Docker Compose file. Are you sure that they aren't required?
Also please attach a video of using Docker Compose to setup the project, and then give a show tour of the talawa-admin to verify all the functionalities are working as expected.
Hi @EshaanAgg, thanks for the review, changed node t version to lts in Dockerfile as you suggested |
@EshaanAgg thanks for the review, I have updated the docs so that the user would first run the setup script so that .env file will get populated, and then the user will use docker compose. |
What is the scope of this PR? Are the docker containers only needed for frontend people to not have to set up everything by themselves? If that's the case, then there have to be some sane defaults and configuration so that frontend people don't have to manually touch the internals of whatever's running inside the containers to accomodate for their use cases. Or would they also want to interface with or make changes to the api and databases and have it reflect in their running instances of the docker containers? You would need to take care of stuff like container server sync/restart on code changes, graceful shutdowns/restarts, interfacing with/manipulating the temporary/persistent data within mongodb/redis databases. And of course documentation for working with the containers. These are some considerations, write the docker configuration accordingly. |
I think this pr will be helpful in two cases - helping frontend people to setup backend easily and secondly for creating a CI/CD pipeline to deploy this repo easily as we can create images of talawa api and push to docker hub on commits to master branch that can be easily pulled by our hosting. This docker compose file takes cares of shutdowns/restarts too as mongodb or redis data modified by user is saved in persistent docker volumes created by compose file. I think we can do the same for talawa-admin too so that we can add github actions to automatically deploy both talawa-api and admin to docker hub. For the changes made by dev in talawa api during development, it will be visible once the dev restart the containers using docker compose that will make new changes to docker image while also retaining database data modified by user in docker volume. For the default configuration suggestion, I would like to point out that this process does not simplfy any configuration instead creates docker instances of mongodb,redis and talwa-api backed so that the frontend end/Flutter dev doesn't need to install them locally on his machine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good. Please add a video demo of using the Docker container to run the Talawa Service. Will approve the PR after to ensure nothing breaks.
I have added the demo video of running talawa-api using docker and also included working graphql endpoints should I add talawa admin working demo too? |
@EshaanAgg Please have a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Next time please add the video in the pull request description itself. Thanks for your contribution and good work!
Will remember to add a demo video in the description itself next time, thank you for helping! |
…sFoundation#1418) * feat: Dockerfile and docker-compose.yaml created * Added env vars to compose file and modified setup.ts for docker-compose * Added Docker Installation Steps to Documentation * Updated Table of Contents for INSTALLATION.md * docs: made a separate section for docker installation * refactor: Skips MonogoURL Setup if docker installation in setup.ts * refactor: Changed node:19 to node:lts in Dockerfile
…sFoundation#1418) * feat: Dockerfile and docker-compose.yaml created * Added env vars to compose file and modified setup.ts for docker-compose * Added Docker Installation Steps to Documentation * Updated Table of Contents for INSTALLATION.md * docs: made a separate section for docker installation * refactor: Skips MonogoURL Setup if docker installation in setup.ts * refactor: Changed node:19 to node:lts in Dockerfile
What kind of change does this PR introduce?
Added Dockerfile and docker-compose file for easy setup locally
Issue Number:
Fixes PalisadoesFoundation/talawa-admin#974
Did you add tests for your changes?
No
Snapshots/Videos:
N/A
If relevant, did you update the documentation?
Yes Updated the Docs
Does this PR introduce a breaking change?
No
Summary:
I have created method to quickly setup talawa-api on local machines using Docker. The Dockerfile will create a docker image of the talawa-api. Then docker-compose will pull mongodb and redis images from docker hub and will start them locally as containers with talawa-api container.
Other information
This PR is Releated to this issue - PalisadoesFoundation/talawa-admin#974
Have you read the contributing guide?
Yes