Added Docker Support to Talawa Admin, Fixes #1075 #1726
Conversation
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. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1726 +/- ##
===========================================
- Coverage 98.43% 97.55% -0.88%
===========================================
Files 221 177 -44
Lines 5945 4385 -1560
Branches 1734 1258 -476
===========================================
- Hits 5852 4278 -1574
- Misses 87 103 +16
+ Partials 6 4 -2 ☔ View full report in Codecov by Sentry. |
|
@Tajcore @beingnoble03 Can you review this PR? |
|
This is an update on the PR merging freeze:
We decided to do this at the beginning of the weekend to give us all time to adjust PR code and create bug fixes that may arise. Update your code at or after midnight GMT on the morning of March 23, 2024. (5:30am IST). If your PRs have already been approved, request a re-review after fixing the conflicts and refactoring to the new |
The PR merging freeze is lifted.
Background:
|
1 similar comment
The PR merging freeze is lifted.
Background:
|
|
@vasujain275 Can you fix the linting test issue? |
|
@Cioppolo14 Sorry for the late response, I was not well. Can you link me to the issue you are referring to? |
|
|
@vasujain275 Please fix the linting issue. |
Cioppolo14
requested review from
Cioppolo14
and removed request for
Tajcore and
beingnoble03
I tried solving the liniting issue but not able to understand how to solve it, any guidance on how to do it would be really helpful. Otherwise I can close this PR and create a new one if solving the linting issue in this pr is too complex. |
|
This pull request did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
github-actions
bot
added
no-pr-activity
|
Is anything else required for a review of this? |
No, I don't think so |
Okay |
|
@varshith257 Added .dockerignore file, thanks for the suggestion |
|
@vasujain275 The tests aren't running. PTAL |
|
Okay, let me take a look |
WalkthroughThe changes in this update focus on adding Docker support for the Talawa-Admin project. This includes creating a Dockerfile and a docker-compose.yml configuration to streamline deployment and ensure consistency across environments. Additionally, updates were made to Changes
Assessment Against Linked Issues
Poem
Tip Early access features
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Fixed the conflicting files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
Dockerfile (1)
1-1: Consider pinning the Node.js image to a specific minor version to ensure consistent builds.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (5)
- .dockerignore (1 hunks)
- .eslintignore (1 hunks)
- Dockerfile (1 hunks)
- docker-compose.yml (1 hunks)
- package.json (3 hunks)
Files skipped from review due to trivial changes (3)
- .dockerignore
- .eslintignore
- docker-compose.yml
Additional context used
Hadolint
Dockerfile
[warning] 7-7: Pin versions in npm. Instead of
npm install <package>usenpm install <package>@<version>
[info] 9-9: Multiple consecutive
RUNinstructions. Consider consolidation.
[info] 11-11: Multiple consecutive
RUNinstructions. Consider consolidation.
Additional comments not posted (5)
Dockerfile (3)
3-3: This is a good practice to set a working directory.
13-15: Proper use ofEXPOSEandCMDto set up the container's exposed port and default command.
5-5: Ensure that the.dockerignorefile is properly configured to avoid unnecessary files being copied into the Docker image.Verification successful
The
.dockerignorefile is properly configured to exclude unnecessary files from being copied into the Docker image.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if .dockerignore is properly set up. cat .dockerignoreLength of output: 388
package.json (2)
Line range hint
25-35: Ensure that all new dependencies are necessary and correctly versioned to avoid potential conflicts and ensure reproducibility.
74-75: The addition of Docker-related scripts enhances the developer experience by simplifying Docker operations.
|
@varshith257 PTAL here too. We need to get this functionality working so that we have a live demo instance of the API and Admin in the next few weeks. This will help with us getting grants for internships as well as new contributors |
|
|
||
| COPY . . | ||
|
|
||
| RUN npm install -g typescript |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (1)
- package.json (3 hunks)
Additional comments not posted (3)
package.json (3)
25-25: Verify the usage ofdotenv.Ensure that the
dotenvdependency is used appropriately to manage environment variables, especially in the Docker configuration.Verification successful
The usage of
dotenvhas been verified.The
dotenvdependency is used in the following files to manage environment variables:
scripts/config-overrides/custom_build.jsscripts/config-overrides/custom_start.jsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `dotenv` in the codebase. # Test: Search for the usage of `dotenv`. Expect: Usage in Docker configuration or environment setup scripts. rg --type python $'dotenv'Length of output: 56
Script:
#!/bin/bash # Description: Verify the usage of `dotenv` in the codebase. # Test: Search for the usage of `dotenv`. Expect: Usage in Docker configuration or environment setup scripts. rg --type js 'dotenv'Length of output: 159
75-75: Verify the correctness and necessity of thedx-prodscript.Ensure that the
dx-prodscript correctly runs Docker compose operations and aligns with the objectives of the PR.Verification successful
The
dx-prodscript is correctly set up to build and run the Docker compose configuration defined indocker-compose.yml.
- The script correctly references the
docker-compose.ymlfile.- The
docker-compose.ymlfile includes appropriate build instructions and environment variables for theappservice.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness and necessity of the `dx-prod` script. # Test: Check if the `dx-prod` script correctly runs Docker compose operations. docker compose -f docker-compose.yml up -d --buildLength of output: 126
Script:
#!/bin/bash # Find the docker-compose.yml file and display its contents fd -H -I -t f -i 'docker-compose.yml' --exec cat {} # Search for references to the `dx-prod` script in the codebase rg 'dx-prod'Length of output: 358
35-35: Verify the necessity of updatinginquirer.Ensure that the update of
inquirerfrom^8.0.0to^8.2.6is necessary and does not introduce compatibility issues.
|
Please fix the failing workflow |
|
Should this PR be closed? It's been open a long time. |
Forgot about this PR entirely, my bad. I glanced through it and it is a much needed change to our docker files of projects. Let me fix the conflicts and finalize this for review by this or at max next weekend. |
|
Closing due to inactivity. |
What kind of change does this PR introduce?
This PR add Docker Support to Talawa Admin.
Issue Number:
Did you add tests for your changes?
N/A
Snapshots/Videos:
If relevant, did you update the documentation?
N/A
Summary
1. Introduction of Docker Support:
2. Consideration of Docker Methods:
Create React Appand react scripts for the build process.3. Proposal for Build Process Improvement:
4. Planned Actions:
5. Current Status:
Does this PR introduce a breaking change?
No
Other information
You can test the current Docker Installtion process by using -
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
dx-prod).Chores
.dockerignoreand.eslintignorefiles to optimize builds and linting.package.jsonwith new and updated dependencies (dotenv,inquirer).Configuration
docker-compose.ymlto configure the application service, set environment variables, and expose port 4321.